Add OpCode for activity started and stop in OpenTelemetry-Sdk eventsource #6046
Conversation
The committers listed above are authorized under a signed CLA. |
github-actions
bot
added
the
pkg:OpenTelemetry
| @@ -185,13 +185,13 @@ public void ObservableInstrumentCallbackException(string exception) | |||
| this.WriteEvent(16, exception); | |||
| } | |||
|
|
|||
| [Event(24, Message = "Activity started. Name = '{0}', Id = '{1}'.", Level = EventLevel.Verbose)] | |||
| [Event(24, Message = "Activity started. Name = '{0}', Id = '{1}'.", Level = EventLevel.Verbose, Opcode = EventOpcode.Start)] | |||
There was a problem hiding this comment.
Can you elaborate why is this important? This code is in SDK's internal logging, and there is no guarantee made on the internal log format. (In fact, internal logging may move to ILogger in the future).
There was a problem hiding this comment.
Hi @cijothomas Thanks for the asking. This is to align with the analyzers like perf view (TraceEvent):
Event source events without OpCode of Start/Stop will be treated as exceptions. Having OpCode marking the start/stop of a request makes it closer to meet the conventions.
And that helps with EventPipe trace / dotnet-trace to better understand those events.
There was a problem hiding this comment.
Doing it this way will work fine, but I'll also suggest there is naming convention where if you name events with the suffix "Start" and "Stop" then the opcodes are inferred automatically. You don't have to use the naming convention but you will be a little more aligned with names other events on other providers if you do.
There was a problem hiding this comment.
@noahfalk Thanks for pointing that out. That's actually what I really want! @cijothomas, how much impact do you know we will have by changing the name of the events to ActivityStart / ActivityStop from ActivityStarted / ActivityStopped?
Hi maintainers,
OpenTelemetry-Sdkprovides activity started / stopped events. How about addingOpCodeon those events, making it easier to be discovered by the analysis tools?Changes
Add opcode for activity start / stop to
OpenTelemetry-Sdkevent source.Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes