-
Notifications
You must be signed in to change notification settings - Fork 848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prototype: User facing logs API (including emit event) #6761
base: main
Are you sure you want to change the base?
Conversation
be202ea
to
26785ca
Compare
Let me know when you want me to give this a proper review |
60f5a2a
to
51c8611
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6761 +/- ##
============================================
- Coverage 90.49% 90.26% -0.23%
Complexity 6599 6599
============================================
Files 731 733 +2
Lines 19735 19789 +54
Branches 1935 1935
============================================
+ Hits 17859 17863 +4
- Misses 1285 1336 +51
+ Partials 591 590 -1 ☔ View full report in Codecov by Sentry. |
I think it's ready for a basic review of the proposed API surface. I'll move it to the incubator module and add tests later on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it, no real surprises here.
api/all/src/main/java/io/opentelemetry/api/logs/EventBuilder.java
Outdated
Show resolved
Hide resolved
api/all/src/main/java/io/opentelemetry/api/logs/EventBuilder.java
Outdated
Show resolved
Hide resolved
5f834d4
to
c11689e
Compare
I sure would like a convenience method that (I think) takes a name and some attributes, and skips the builder. Or however we spell Attributes for the event API. |
so like this?
|
Continuing the work of implementing [OTEP 265](https://github.com/open-telemetry/oteps/blob/main/text/0265-event-vision.md), this PR adds the EmitEvent method to the Log API. Deprecating the experimental Event API and SDK can be handled in a follow up PR. Java prototype: open-telemetry/opentelemetry-java#6761 --------- Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com> Co-authored-by: Robert Pająk <pellared@hotmail.com>
We had an extensive discussion about this in the last Java SIG meeting, and it became very clear that it's not clear what should be attributes and what should be body, and how we even explain the difference to people, and how we should tell people to use them. Whatever the results of that are, we should have an API that makes it really easy to do the recommended thing. |
It's probably more than API design problem - I don't think we have a clear guidance from semconv perspective on what should go to attributes vs body. |
I feel like we might be getting the cart ahead of the horse a bit, if we're not even sure (as a community) that we know how people are supposed to even be using the new APIs. For instance, if the body is the thing people should be thinking about, we can make that the front-and-center thing, but if it's attributes, then we should make that the easy thing. Until that's decided, I'm guessing we won't know how to build a good user-facing API. |
c11689e
to
59862e4
Compare
I think it would be helpful at this point to move forward with the prototype / incubating API so people (@breedx-splk) can start using it as part of the effort to push the horse forward |
Body is the front and center thing. Evidence is that the semantic conventions are defining events in terms of fields of the body and not attributes. I think there's still outstanding question for end users producing custom events on when its appropriate to use attributes, but IMO its clear that body is priority. |
We have evidence in the both ways (feature-flags). The genai conventions you mentioned turned out problematic - some of the body fields are essential when querying things, but it's much harder to do using body fields. I'll propose to promote them to attributes. Plus there will be more indexable things that would go to attributes - see https://github.com/microsoft/opentelemetry-semantic-conventions/blob/e7276ff1bebb5914048d4843eeb78f20abe3de00/docs/gen-ai/gen-ai-events.md#event-gen_aisystemmessage as an example. We don't really know how people would use events - we are still clarifying our own understanding of what goes where and why, but the answer seems that we need both body and attributes equally. |
Just copy pasted over from the (to be previous) Event API as a first pass...