opentelemetry-java icon indicating copy to clipboard operation
opentelemetry-java copied to clipboard

Prototype: User facing logs API (including emit event)

Open trask opened this issue 1 year ago • 3 comments

Just copy pasted over from the (to be previous) Event API as a first pass...

trask avatar Oct 02 '24 22:10 trask

Let me know when you want me to give this a proper review

jack-berg avatar Oct 02 '24 22:10 jack-berg

Codecov Report

Attention: Patch coverage is 5.55556% with 51 lines in your changes missing coverage. Please review.

Project coverage is 90.26%. Comparing base (74579aa) to head (c11689e).

Files with missing lines Patch % Lines
...ava/io/opentelemetry/sdk/logs/SdkEventBuilder.java 0.00% 26 Missing :warning:
.../java/io/opentelemetry/api/logs/DefaultLogger.java 11.11% 8 Missing :warning:
...etry/api/incubator/logs/ExtendedDefaultLogger.java 11.11% 8 Missing :warning:
...main/java/io/opentelemetry/sdk/logs/SdkLogger.java 16.66% 5 Missing :warning:
.../main/java/io/opentelemetry/api/OpenTelemetry.java 0.00% 3 Missing :warning:
...rc/main/java/io/opentelemetry/api/logs/Logger.java 0.00% 1 Missing :warning:
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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 02 '24 22:10 codecov[bot]

Let me know when you want me to give this a proper review

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.

trask avatar Oct 03 '24 01:10 trask

I sure would like a convenience method that (I think) takes a name and some attributes, and skips the builder. logger.emit("paymentReceived", Attributes.of("amount", 3.14, "currency", "Dollars"))

Or however we spell Attributes for the event API.

jkwatson avatar Oct 31 '24 01:10 jkwatson

I sure would like a convenience method that (I think) takes a name and some attributes, and skips the builder. logger.emit("paymentReceived", Attributes.of("amount", 3.14, "currency", "Dollars"))

Or however we spell Attributes for the event API.

so like this?

logger.emit("paymentReceived", Value.builder().put("amount", 3.14).put("currency", "Dollars").build())

trask avatar Oct 31 '24 03:10 trask

I sure would like a convenience method that (I think) takes a name and some attributes, and skips the builder. logger.emit("paymentReceived", Attributes.of("amount", 3.14, "currency", "Dollars")) Or however we spell Attributes for the event API.

so like this?

logger.emit("paymentReceived", Value.builder().put("amount", 3.14).put("currency", "Dollars").build())

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.

jkwatson avatar Nov 02 '24 19:11 jkwatson

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.

lmolkova avatar Nov 03 '24 18:11 lmolkova

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.

jkwatson avatar Nov 04 '24 02:11 jkwatson

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.

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

trask avatar Dec 04 '24 21:12 trask

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.

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.

jack-berg avatar Dec 04 '24 23:12 jack-berg

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.

lmolkova avatar Dec 04 '24 23:12 lmolkova

Closing in favor of #7012

See https://github.com/open-telemetry/opentelemetry-specification/issues/4357 to track the future of an ergonomic event API

trask avatar Jan 17 '25 17:01 trask