feat: Added support for OTEL logs API #3228
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3228 +/- ##
==========================================
- Coverage 97.67% 97.62% -0.05%
==========================================
Files 340 344 +4
Lines 51436 51683 +247
==========================================
+ Hits 50239 50458 +219
- Misses 1197 1225 +28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
test/integration/otel/logs.test.js
Outdated
| assert.equal(nrShippedLogs.length, 1) | ||
| assert.equal(nrShippedLogs[0].common.attributes['entity.guid'], 'guid-123456') | ||
| const log = nrShippedLogs[0].logs[0] | ||
| assert.equal(log['entity.guid'], 'guid-123456') |
There was a problem hiding this comment.
The common attributes should not exist on the logs as well. I'd also add a test that runs this in a transaction so it can add the trace.id and span.id which should exist on the individual logs
lib/otel/logs/bootstrap-logs.js
Outdated
| // TODO: if we decide to support local decorating, implement it here | ||
|
|
||
| if (isLogForwardingEnabled(agent.config, agent) === true) { | ||
| const meta = agent.getLinkingMetadata() |
There was a problem hiding this comment.
if you pass in true this will omit the service level linking which should be done to avoid logging entity.guid, service etc in every log line
There was a problem hiding this comment.
Also, you may want to add some versioned tests for an otel js contrib library. Looking at their instrumentation they add trace.id, span.id and traceFlags so there will be some overlap
| const fakeCert = require('#testlib/fake-cert.js') | ||
| const helper = require('#testlib/agent_helper.js') | ||
|
|
||
| test.beforeEach(async (ctx) => { |
There was a problem hiding this comment.
you should add tests that disable application logging metrics and log forwarding and assert the proper data
| isLogForwardingEnabled, | ||
| isMetricsEnabled, | ||
| incrementLoggingLinesMetrics | ||
| } = require('#agentlib/util/application-logging.js') |
There was a problem hiding this comment.
I forgot to point this out earlier: I'm using the #agentlib alias here. I think this will be a good case for testing the usage of that alias within the actual agent code. Currently, we only utilize it in test code. Team members are wanting to utilize it agent code, and this whole feature is in-development, which I think provides a good testing ground.
| @@ -17,5 +17,10 @@ exports.config = { | |||
| detect_azure: false, | |||
| detect_gcp: false, | |||
| detect_docker: false | |||
| } | |||
| }, | |||
| instrumentation: { | |||
There was a problem hiding this comment.
i'd move this to the test that relies on this being disabled
| const agent = helper.instrumentMockedAgent({ | ||
| opentelemetry_bridge: { | ||
| enabled: true, | ||
| logs: { enabled: true } |
There was a problem hiding this comment.
you need to also enable traces in otel bridge at the moment. this is because the context manager and trace propagator are registered there. The reason you aren't seeing any logs on tx is because of the lack of assigning the global context manager
This PR resolves #3229.