Skip to content

fix: generated semconv helpers skipping attributes#7964

Merged
dmathieu merged 8 commits intoopen-telemetry:mainfrom
victoraugustolls:fix/metrics-generation-7938
Mar 4, 2026
Merged

fix: generated semconv helpers skipping attributes#7964
dmathieu merged 8 commits intoopen-telemetry:mainfrom
victoraugustolls:fix/metrics-generation-7938

Conversation

@victoraugustolls
Copy link
Copy Markdown
Contributor

@victoraugustolls victoraugustolls commented Feb 26, 2026

This is related to issue #7938.

The Bug

Semconv code generator Jinja2 template (instrument.j2) had an early return optimization that skipped required attributes when no extra optional attributes were passed.

The root cause seemed to be two macros in semconv/templates/registry/go/instrument.j2:

  • add_method_with_optional (for counters/updowncounters)
  • record_method_with_optional (for histograms/gauges)

Both had if len(attrs) == 0 { m.Inst.Add(ctx, incr); return }, which bypassed required attributes entirely.

The Fix

I added a conditional check to verify when required attributes exist (req_attr | length > 0), the early return now passes them via metric.WithAttributes(...). When there are no required attributes, the original fast path is preserved.

I also regenerated the semconv v1.39.0, but if thats not desired due to existing clients, let me know. It is my first time contributing here and any feedback is appreciated!

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.6%. Comparing base (e2305d2) to head (7caf127).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #7964   +/-   ##
=====================================
  Coverage   81.6%   81.6%           
=====================================
  Files        304     304           
  Lines      23384   23384           
=====================================
+ Hits       19086   19090    +4     
+ Misses      3909    3905    -4     
  Partials     389     389           

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

Copy link
Copy Markdown
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Can you please add a changelog entry for v1.39.0 (no need for v1.40.0 as this was not released yet) under "Fixed" section?

@victoraugustolls victoraugustolls force-pushed the fix/metrics-generation-7938 branch 2 times, most recently from 9c508fc to 356af2f Compare March 3, 2026 19:07
@victoraugustolls
Copy link
Copy Markdown
Contributor Author

Hey @pellared! Updated the changelog, rebased with main and regenerated the code just in case

@victoraugustolls
Copy link
Copy Markdown
Contributor Author

Want me to include the regenerated v1.40.0 with the fix in this PR?

@MrAlias
Copy link
Copy Markdown
Contributor

MrAlias commented Mar 3, 2026

Want me to include the regenerated v1.40.0 with the fix in this PR?

yes

@victoraugustolls victoraugustolls force-pushed the fix/metrics-generation-7938 branch from 97ae6dd to c521e65 Compare March 4, 2026 03:49
@victoraugustolls
Copy link
Copy Markdown
Contributor Author

yes

Done @MrAlias !

@pellared
Copy link
Copy Markdown
Member

pellared commented Mar 4, 2026

This needs an approval from an approver who is not working for Splunk.

@pellared pellared added this to the v1.42.0 milestone Mar 4, 2026
@dmathieu dmathieu merged commit edba765 into open-telemetry:main Mar 4, 2026
33 of 34 checks passed
pellared added a commit that referenced this pull request Mar 6, 2026
### Added

- Add `go.opentelemetry.io/otel/semconv/v1.40.0` package.
The package contains semantic conventions from the `v1.40.0` version of
the OpenTelemetry Semantic Conventions.
See the [migration documentation](./semconv/v1.40.0/MIGRATION.md) for
information on how to upgrade from
`go.opentelemetry.io/otel/semconv/v1.39.0`. (#7985)
- Add `Err` and `SetErr` on `Record` in `go.opentelemetry.io/otel/log`
to attach an error and set record exception attributes in
`go.opentelemetry.io/otel/log/sdk`. (#7924)

### Changed

- `TracerProvider.ForceFlush` in `go.opentelemetry.io/otel/sdk/trace`
joins errors together and continues iteration through SpanProcessors as
opposed to returning the first encountered error without attempting
exports on subsequent SpanProcessors. (#7856)

### Fixed

- Fix missing `request.GetBody` in
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` to
correctly handle HTTP2 GOAWAY frame. (#7931)
- Fix semconv v1.39.0 generated metric helpers skipping required
attributes when extra attributes were empty. (#7964)
- Preserve W3C TraceFlags bitmask (including the random Trace ID flag)
during trace context extraction and injection in
`go.opentelemetry.io/otel/propagation`. (#7834)

### Removed

- Drop support for [Go 1.24]. (#7984)
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.

4 participants