Skip to content

feat(sdk-logs): implement log creation sdk metrics#6433

Merged
trentm merged 9 commits intoopen-telemetry:mainfrom
anuraaga:logger-metrics
Apr 2, 2026
Merged

feat(sdk-logs): implement log creation sdk metrics#6433
trentm merged 9 commits intoopen-telemetry:mainfrom
anuraaga:logger-metrics

Conversation

@anuraaga
Copy link
Copy Markdown
Contributor

@anuraaga anuraaga commented Feb 20, 2026

Which problem is this PR solving?

I am helping to implement SDK internal metrics

https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/

This adds log creation metrics

Very similar to #6213 which was for traces.

/cc @trentm

Short description of the changes

Implements metrics for creation of logs per semconv

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Unit tests

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@anuraaga anuraaga requested a review from a team as a code owner February 20, 2026 08:02
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.74%. Comparing base (1b8a6a2) to head (0937288).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6433   +/-   ##
=======================================
  Coverage   95.73%   95.74%           
=======================================
  Files         369      371    +2     
  Lines       12497    12513   +16     
  Branches     2959     2962    +3     
=======================================
+ Hits        11964    11980   +16     
  Misses        533      533           
Files with missing lines Coverage Δ
...imental/packages/opentelemetry-sdk-node/src/sdk.ts 95.93% <100.00%> (+0.02%) ⬆️
experimental/packages/sdk-logs/src/Logger.ts 100.00% <100.00%> (ø)
...xperimental/packages/sdk-logs/src/LoggerMetrics.ts 100.00% <100.00%> (ø)
...perimental/packages/sdk-logs/src/LoggerProvider.ts 100.00% <ø> (ø)
...sdk-logs/src/internal/LoggerProviderSharedState.ts 97.29% <100.00%> (+0.63%) ⬆️
experimental/packages/sdk-logs/src/semconv.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread experimental/packages/sdk-logs/src/LoggerMetrics.ts
Comment thread experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Outdated
Comment thread experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Outdated
Comment thread experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Outdated
Comment thread experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Outdated
Comment thread experimental/packages/sdk-logs/src/LoggerMetrics.ts Outdated
...logRecord,
}
);
this._sharedState.loggerMetrics.emitLog();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A possible subtlety that isn't considered by the spec, because of timing: Since the SDK metrics were added to semconv, support for Logger#emit filtering log records based on possible Logger config was added. https://opentelemetry.io/docs/specs/otel/logs/sdk/#emit-a-logrecord

Status: Development Before processing a log record, the implementation MUST apply the filtering rules defined by the LoggerConfig (in case Enabled was not called prior to emitting the record):

  1. Minimum severity: If the log record’s SeverityNumber is specified (i.e. not 0) and is less than the configured minimum_severity, the log record MUST be dropped.

  2. Trace-based: If trace_based is true, and if the log record has a SpanId and the TraceFlags SAMPLED flag is unset, the log record MUST be dropped.

I think it is worth a follow-up on the spec/semconv whether the intention is that the otel.sdk.log.created metric is counted before or after (as you have it here) that filtering. @anuraaga Is there an implementation of this metric in OTel Java? If so, what is done there?

(Aside: This Logger#emit is currently missing checking loggerConfig.disabled. I'll open an issue for that separately.)

Copy link
Copy Markdown
Contributor Author

@anuraaga anuraaga Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - Java is implemented in the same way, after the enabled check (organically, I didn't decide one way or the other not realizing the caveat at the time)

https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java#L152

Filed open-telemetry/semantic-conventions#3506 in semconv to follow up.

For now, it seems fine to go with post-filtering because with no way to identify filtered vs not via attribute or separate metric, mixing the two is probably less useful.

Comment thread experimental/packages/sdk-logs/src/semconv.ts Outdated
Comment thread package-lock.json Outdated
@trentm trentm changed the title feat(sdk-logs): implement log creation metrics feat(sdk-logs): implement log creation sdk metrics Feb 27, 2026
Copy link
Copy Markdown
Contributor

@trentm trentm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just the two nits... caused by my taking too long to review. Approving, assuming those will be updated.

Comment thread CHANGELOG.md Outdated
Comment thread experimental/packages/sdk-logs/package.json Outdated
Comment thread package-lock.json
"dev": true,
"libc": [
"glibc"
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Say what? The "libc" field in package-lock is a new one to me.

You don't need to do anything for this comment. Just remarking on it.
The ways of the npm are mysterious.

The "libc" field in package.json was added in npm v11. So my guess is that you ran npm install with an npm v10. All good, I don't think it matters here.

@anuraaga
Copy link
Copy Markdown
Contributor Author

anuraaga commented Apr 2, 2026

Thanks @trentm - updated

@trentm trentm added this pull request to the merge queue Apr 2, 2026
Merged via the queue into open-telemetry:main with commit 7b7f44f Apr 2, 2026
27 checks passed
@otelbot
Copy link
Copy Markdown
Contributor

otelbot Bot commented Apr 2, 2026

Thank you for your contribution @anuraaga! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants