Semantic sugar Event API #4961

jack-berg · 2022-11-15T22:44:35Z

Implements the recent change to to the specification to split out the Event API: open-telemetry/opentelemetry-specification#2941

Event API is split out into new module :extensions:event-api which will publish artifact with id opentelemetry-extension-event-api.

codecov · 2022-11-15T23:02:03Z

Codecov Report

Base: 91.05% // Head: 91.05% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (0a1a692) compared to base (08e19e3).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 0a1a692 differs from pull request most recent head e95f296. Consider uploading reports for the commit e95f296 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #4961      +/-   ##
============================================
- Coverage     91.05%   91.05%   -0.01%     
- Complexity     4805     4834      +29     
============================================
  Files           544      547       +3     
  Lines         14305    14446     +141     
  Branches       1367     1393      +26     
============================================
+ Hits          13026    13154     +128     
- Misses          882      891       +9     
- Partials        397      401       +4

Impacted Files	Coverage Δ
...java/io/opentelemetry/api/logs/LoggerProvider.java	`100.00% <ø> (ø)`
...io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java	`97.43% <ø> (ø)`
...main/java/io/opentelemetry/sdk/logs/SdkLogger.java	`100.00% <ø> (+10.00%)`	⬆️
.../java/io/opentelemetry/api/events/EventLogger.java	`100.00% <100.00%> (ø)`
.../java/io/opentelemetry/api/logs/DefaultLogger.java	`100.00% <100.00%> (ø)`
.../opentelemetry/api/logs/DefaultLoggerProvider.java	`100.00% <100.00%> (+8.33%)`	⬆️
...va/io/opentelemetry/sdk/logs/SdkLoggerBuilder.java	`100.00% <100.00%> (ø)`
...telemetry/sdk/trace/export/BatchSpanProcessor.java	`91.72% <0.00%> (-0.69%)`	⬇️
...opentelemetry/extension/aws/AwsXrayPropagator.java	`90.64% <0.00%> (ø)`
...metry/extension/aws/AwsConfigurablePropagator.java	`0.00% <0.00%> (ø)`
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

api/logs/src/main/java/io/opentelemetry/api/events/EventLogger.java

mateuszrzeszutek · 2022-11-16T09:21:48Z

The last option is actually pretty attractive to me since a the Event API is pretty different from the other stuff we've published in opentelemetry-api. Might make sense to think of it as an extension.

👍 💯

api/logs/src/main/java/io/opentelemetry/api/events/EventLogger.java

…y-java into syntactic-sugar-event-api

jkwatson · 2022-11-17T20:26:18Z

extensions/event-api/src/main/java/io/opentelemetry/extension/event/EventLogger.java

+ * }
+ * }</pre>
+ */
+public final class EventLogger {


During the java SIG meeting, it was widely agreed that this name is not great, and that EventEmitter might be preferable.

Perhaps this should be made an interface.

Perhaps this should be made an interface.

That suggests alternative implementations. The current was to use one of these is something like:

LoggerProvider loggerProvider = ... // Configure a logger provider Logger logger = loggerProvider.get("my-logger"); EventEmitter eventEmitter = EventLogger.create(logger, "my-event-domain");

What kind of workflow did you have in mind that would allow for alternative implementations to be plugged in?

I know that we are far away from implementing events independently from logs, and this might even never happen. But I don't agree that alternative implementations must exist in order to define an interface.
I hope that a new interface will help emphasize that events are conceptually different than logs.
And the new interface will at least allow us to provide a different implementation in the future without any breaking changes - even if this is hypothetical at this point.

trask · 2022-11-17T20:56:43Z

one thing I liked about @jkwatson's question about making Event API a "full-fledged" API with providers, etc, is I think that would allow the API surface not to mention logs anywhere

PeterF778 · 2022-11-21T16:23:29Z

extensions/event-api/src/main/java/io/opentelemetry/extension/event/EventLogger.java

+   *     particular event domain, event name defines a particular class or type of event.
+   * @return an {@link EventLogger} instance
+   */
+  public static EventLogger create(Logger logger, String eventDomain) {


I suggest naming it fromLogger.

jack-berg · 2022-11-21T19:36:48Z

one thing I liked about @jkwatson's question about making Event API a "full-fledged" API with providers, etc, is I think that would allow the API surface not to mention logs anywhere

Would you extend that to having a dedicated event SDK implementation and OTLP representation as well? If so, that's quite a lot of extra code to maintain which would be nearly identical to logs. If not, then users are still eventually confronted with the notion of logs and events being the same thing - just either when they configure the SDK or wherever they receive OTLP.

I like the idea of the obvious event API dependency on the log API because it makes it clear as far upstream as possible that events are just a log record with specific characteristics. I wouldn't mind if events were different at the API, SDK, and OTLP level, but think that's a longshot. So in the absence of completely separate log and event signals, better to make their relationship obvious rather than hiding it in the SDK or in OTLP for users to be surprised by later.

trask · 2022-11-21T20:09:06Z

Would you extend that to having a dedicated event SDK implementation and OTLP representation as well?

No

If not, then users are still eventually confronted with the notion of logs and events being the same thing - just either when they configure the SDK or wherever they receive OTLP.

For Java agent users, they just point to the OTLP ingestion endpoint, I'm not sure they ever have to be confronted with the notion that logs and events are the same thing at the transport layer.

jack-berg · 2022-11-21T20:35:05Z

For Java agent users, they just point to the OTLP ingestion endpoint, I'm not sure they ever have to be confronted with the notion that logs and events are the same thing at the transport layer.

Java agent users don't need to know about the API or SDK either. So I suppose its instrumentation authors and end users directly using the API / SDK that we're designing around.

trask · 2022-11-21T20:39:17Z

Java agent users don't need to know about the API or SDK either

my particular selfish interest is customers using the Java agent who want to emit custom events

github-actions · 2022-12-05T22:29:02Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

jkwatson · 2022-12-06T02:56:20Z

For Java agent users, they just point to the OTLP ingestion endpoint, I'm not sure they ever have to be confronted with the notion that logs and events are the same thing at the transport layer.

Java agent users don't need to know about the API or SDK either. So I suppose its instrumentation authors and end users directly using the API / SDK that we're designing around.

Java agent users should definitely know about the API. We should encourage the creation of custom spans, metrics and events, all of which require knowledge of the APIs.

jack-berg · 2022-12-06T15:48:47Z

Java agent users should definitely know about the API. We should encourage the creation of custom spans, metrics and events, all of which require knowledge of the APIs.

Agreed. Java agent users don't need to know about the API / SDK, but many will.

That statement originated further up in the conversation here, where I argued that if events end up using the OTLP log transport, users have to eventually be confronted with the fact that events are just a particular type of log. Better to be confronted as soon as possible in the API than in the SDK:

I like the idea of the obvious event API dependency on the log API because it makes it clear as far upstream as possible that events are just a log record with specific characteristics.

jkwatson · 2022-12-06T17:33:34Z

Java agent users should definitely know about the API. We should encourage the creation of custom spans, metrics and events, all of which require knowledge of the APIs.

Agreed. Java agent users don't need to know about the API / SDK, but many will.

That statement originated further up in the conversation here, where I argued that if events end up using the OTLP log transport, users have to eventually be confronted with the fact that events are just a particular type of log. Better to be confronted as soon as possible in the API than in the SDK:

I like the idea of the obvious event API dependency on the log API because it makes it clear as far upstream as possible that events are just a log record with specific characteristics.

I've been trying to argue the opposite of this. I don't think anyone (aside from operators) should have to care about the weird fact that we happened to use the log datamodel to represent events. When writing instrumentation that generates events, that seems like it should be considered an implementation detail that would depend on how the operator's choice of backend has decided to deal with them.

jack-berg · 2022-12-06T17:38:09Z

I've been trying to argue the opposite of this. I don't think anyone (aside from operators) should have to care about the weird fact that we happened to use the log datamodel to represent events.

Right so would you imagine a fully separate event SDK in addition to a separate event API? If only the API is to be separate, then users configuring the log SDK have to be aware that its handling both logs and events.

jkwatson · 2022-12-06T17:57:55Z

I've been trying to argue the opposite of this. I don't think anyone (aside from operators) should have to care about the weird fact that we happened to use the log datamodel to represent events.

Right so would you imagine a fully separate event SDK in addition to a separate event API? If only the API is to be separate, then users configuring the log SDK have to be aware that its handling both logs and events.

SDK configurers (what I was referring to the "operator" above) definitely need to know. I would hope that API users wouldn't need to be concerned with this implementation detail.

github-actions · 2022-12-20T18:30:33Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

github-actions · 2023-01-03T19:27:07Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Semantic sugar Event API

0a1a692

jack-berg requested a review from a team November 15, 2022 22:44

jkwatson reviewed Nov 16, 2022

View reviewed changes

api/logs/src/main/java/io/opentelemetry/api/events/EventLogger.java Outdated Show resolved Hide resolved

mateuszrzeszutek reviewed Nov 16, 2022

View reviewed changes

api/logs/src/main/java/io/opentelemetry/api/events/EventLogger.java Outdated Show resolved Hide resolved

jack-berg commented Nov 16, 2022

View reviewed changes

api/logs/src/main/java/io/opentelemetry/api/events/EventLogger.java Outdated Show resolved Hide resolved

jack-berg added 4 commits November 16, 2022 17:38

Move event API to extension artifact

2d7cd5c

Simplify event api to eventName and attributes

6676851

Rename to :extension:Event-api

943dd08

Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…

e95f296

…y-java into syntactic-sugar-event-api

mateuszrzeszutek approved these changes Nov 17, 2022

View reviewed changes

jkwatson reviewed Nov 17, 2022

View reviewed changes

PeterF778 reviewed Nov 21, 2022

View reviewed changes

github-actions bot added the Stale label Dec 5, 2022

github-actions bot removed the Stale label Dec 6, 2022

jack-berg mentioned this pull request Dec 16, 2022

Event emitter provider #5049

Merged

github-actions bot added the Stale label Dec 20, 2022

github-actions bot closed this Jan 3, 2023

jack-berg mentioned this pull request Mar 12, 2024

Rename EventEmitter to EventLogger #6290

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Semantic sugar Event API #4961

Semantic sugar Event API #4961

jack-berg commented Nov 15, 2022 •

edited

Loading

codecov bot commented Nov 15, 2022 •

edited

Loading

mateuszrzeszutek commented Nov 16, 2022

jkwatson Nov 17, 2022

PeterF778 Nov 21, 2022

jack-berg Nov 21, 2022

PeterF778 Nov 21, 2022

trask commented Nov 17, 2022

PeterF778 Nov 21, 2022

jack-berg commented Nov 21, 2022

trask commented Nov 21, 2022

jack-berg commented Nov 21, 2022

trask commented Nov 21, 2022

github-actions bot commented Dec 5, 2022

jkwatson commented Dec 6, 2022

jack-berg commented Dec 6, 2022

jkwatson commented Dec 6, 2022

jack-berg commented Dec 6, 2022

jkwatson commented Dec 6, 2022

github-actions bot commented Dec 20, 2022

github-actions bot commented Jan 3, 2023

Semantic sugar Event API #4961

Semantic sugar Event API #4961

Conversation

jack-berg commented Nov 15, 2022 • edited Loading

codecov bot commented Nov 15, 2022 • edited Loading

Codecov Report

mateuszrzeszutek commented Nov 16, 2022

jkwatson Nov 17, 2022

Choose a reason for hiding this comment

PeterF778 Nov 21, 2022

Choose a reason for hiding this comment

jack-berg Nov 21, 2022

Choose a reason for hiding this comment

PeterF778 Nov 21, 2022

Choose a reason for hiding this comment

trask commented Nov 17, 2022

PeterF778 Nov 21, 2022

Choose a reason for hiding this comment

jack-berg commented Nov 21, 2022

trask commented Nov 21, 2022

jack-berg commented Nov 21, 2022

trask commented Nov 21, 2022

github-actions bot commented Dec 5, 2022

jkwatson commented Dec 6, 2022

jack-berg commented Dec 6, 2022

jkwatson commented Dec 6, 2022

jack-berg commented Dec 6, 2022

jkwatson commented Dec 6, 2022

github-actions bot commented Dec 20, 2022

github-actions bot commented Jan 3, 2023

jack-berg commented Nov 15, 2022 •

edited

Loading

codecov bot commented Nov 15, 2022 •

edited

Loading