opentelemetry-js
opentelemetry-js copied to clipboard
Logs/events API
Which problem is this PR solving
This is a draft implementation of the proposed specification for logs/events API.
https://github.com/open-telemetry/opentelemetry-specification/pull/2676
Short description of the changes
For the most part follows the pattern from the trace and metrics APIs. The main things of note are:
- the API of the Logger interface
- Event is defined as an abstract class
The Logger interface has the following methods: createLogRecord(), createEvent(), emit(LogRecord), emit(Event), emitEvent(name, attributes). If we want to have an emit
method that takes an instance of LogRecord / Event, then the API also must provide a method for creating the instance. This is similar to Tracer.startSpan()
or Meter.createCounter()
.
The API defines an abstract class for Event, which
- implements the LogRecord interface
- sets
event.name
andevent.domain
attributes in constructor based on name and domain arguments
Since (if) we have a separate method for emitting an event, and this method should take an instance of an event, then we need to define an interface for an Event. The reason I chose abstract class over interface is because 1) the Event interface would be the same as LogRecord if it was just an interface, and 2) the abstract class can enforce setting name/domain as attributes with the specific keys.
Type of change
Please delete options that are not relevant
- [x] New feature (non-breaking change which adds functionality)
- [x] This change requires a documentation update
How Has This Been Tested?
Checklist:
- [x] Followed the style guidelines of this project
- [x] Unit tests have been added
- [ ] Documentation has been updated
Codecov Report
Merging #3117 (35d4215) into main (5005b0b) will increase coverage by
0.02%
. The diff coverage is100.00%
.
Additional details and impacted files
@@ Coverage Diff @@
## main #3117 +/- ##
==========================================
+ Coverage 93.24% 93.26% +0.02%
==========================================
Files 196 201 +5
Lines 6541 6579 +38
Branches 1373 1379 +6
==========================================
+ Hits 6099 6136 +37
- Misses 442 443 +1
Impacted Files | Coverage Δ | |
---|---|---|
experimental/packages/api-logs/src/NoopLogger.ts | 100.00% <100.00%> (ø) |
|
...mental/packages/api-logs/src/NoopLoggerProvider.ts | 100.00% <100.00%> (ø) |
|
experimental/packages/api-logs/src/api/logs.ts | 100.00% <100.00%> (ø) |
|
...tal/packages/api-logs/src/internal/global-utils.ts | 100.00% <100.00%> (ø) |
|
.../packages/api-logs/src/platform/node/globalThis.ts | 100.00% <100.00%> (ø) |
|
...-trace-base/src/platform/node/RandomIdGenerator.ts | 87.50% <0.00%> (-6.25%) |
:arrow_down: |
I think the folder opentelemetry-api-logs should be renamed to api-logs as we started to omit the opentelemetry-
prefix in new packages.
You're missing the references
entry in the main tsconfig.json
Most recent failure seems to be related to #3238 (let's see if a re-run helps in the meantime to get this unblocked :slightly_smiling_face:)
@dyladan @pichlermarc The codecov check was failing, so I added one more test just to make sure that lower coverage is not caused by this change. Now the HttpsInstrumentation integration test is failing again. Can you please re-run it again?