opentelemetry-js
opentelemetry-js copied to clipboard
feat(api-logs): split out event api
Which problem is this PR solving?
An update to the Logs specification was found during the last pr. It affects the development of code functions. The detailed discussion is here: https://github.com/open-telemetry/opentelemetry-js/pull/3443 This pr implements the latest specifications. The core functionality is to separate out the event api and modify several LogRecord fields to normalize it. Specific details are as follows:
Based on the specification version
https://github.com/open-telemetry/opentelemetry-specification/pull/2941
Content of modification:
- Add the
EventLogger
- Add
EventLoggerProvider
(inherited fromLoggerProvider
, can reuse the provier configuration as a factory for creating theEventLogger
)) - Delete the
LogEvent
(it is no longer needed, just use theLogRecord
data model) - Delete the
emitEvent
method (Migrate to theEventLogger
). - Update the
LoggerOptions
field and deleteeventDomain
(migrate toEventLogger
). AddincludeTraceContext
(a field within the specification that configs the trace context automatically) - Update
LogRecord
fields(based on the latest specification ). In addition,timestamp
is replaced withTimeInput
, since it is unnecessary for developers to generateHrTime
, andTimeInput
is also used in the span structure, which is converted byhrTimeToNanoseconds
in the sdk - Delete the
emitEvent
method inNoopLogger
Finally
There are a few issues to discuss
- Does the event api need to be separated into a separate package? This was also mentioned in the last pr: https://github.com/open-telemetry/opentelemetry-js/pull/3443#issuecomment-1360478844
- How to minimize the impact of experimental changes on schedule? This issue was also discussed at the last SIG meeting. I think we need to implement an sdk and exporter based on a standard first, and then do iterative updates on this basis, otherwise we would never have finished developing the sdk. (The last pr was originally due to a few minor issues I found while implementing the sdk based on an existing api. I modified it on the version of the specification based on (https://github.com/open-telemetry/opentelemetry-specification/pull/2676), but it just happened that the specification had a major modification, so my pr modification could not be incorporated, it makes me a little frustrated 😔)
Type of change
Please delete options that are not relevant.
- [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
Checklist:
- [x] Followed the style guidelines of this project
- [ ] Unit tests have been added
- [ ] Documentation has been updated
Codecov Report
Merging #3501 (f845583) into main (1c3af6c) will decrease coverage by
0.00%
. The diff coverage isn/a
.
Additional details and impacted files
@@ Coverage Diff @@
## main #3501 +/- ##
==========================================
- Coverage 93.74% 93.74% -0.01%
==========================================
Files 249 249
Lines 7593 7592 -1
Branches 1582 1582
==========================================
- Hits 7118 7117 -1
Misses 475 475
Impacted Files | Coverage Δ | |
---|---|---|
experimental/packages/api-logs/src/NoopLogger.ts | 100.00% <ø> (ø) |
|
...erimental/packages/api-logs/src/types/LogRecord.ts | 100.00% <ø> (ø) |
@fuaiyi Apologies again for the delayed response. There are still ongoing discussions about what the Events API should look like (see https://github.com/open-telemetry/opentelemetry-specification/issues/3086). We discussed this in the Client Instrumentation SIG today, and think that we should postpone making significant changes to the API until we have more clarity. I propose that we close this PR for now.
We feel that it is more important to work on the Log SDK implementation. I have started working on that in the past (https://github.com/open-telemetry/opentelemetry-js/pull/3400). And I think you also have an implementation (https://github.com/open-telemetry/opentelemetry-js/pull/3442)? Do you want to (and have capacity to) bring that to conclusion? If not, I can continue on my implementation. If you want to discuss, please attend the Client Instrumentation SIG (or Javascript SIG), or you can also reach out to me on Slack.
We feel that it is more important to work on the Log SDK implementation.
Yes, I also think so. I will close this pr first and make modifications after the event api is confirmed
Do you want to (and have capacity to) bring that to conclusion? If not, I can continue on my implementation. If you want to discuss, please attend the Client Instrumentation SIG (or Javascript SIG), or you can also reach out to me on Slack.
Thank you @martinkuba for your reply, Yes, I have completed the implementation of the sdk, regarding the implementation of sdk, I have brought up pr again in recent days. Then we can discuss it together and see if we can adopt it, this can reduce the development cycle and make the sdk available to everyone quickly. I'm sorry that I can't attend the SIG meeting at present, but I will watch the video of the SIG meeting, and if there is any problem, I will see the discussion. I will try to participate in the SIG meeting in the future