Skip to content

fix(semconv): avoid duplicate metric recording without attributes - #8849

Open
jstar0 wants to merge 4 commits into
open-telemetry:mainfrom
jstar0:fix/8780-semconv-empty-attrs
Open

fix(semconv): avoid duplicate metric recording without attributes#8849
jstar0 wants to merge 4 commits into
open-telemetry:mainfrom
jstar0:fix/8780-semconv-empty-attrs

Conversation

@jstar0

@jstar0 jstar0 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #8780

The retained v1.32.0-v1.34.0 generated metric helpers record through the empty-attributes fast path and then fall through to the general attribute path. This causes histogram helpers to export duplicate count/sum observations and invokes gauge helpers twice.

This updates all 60 affected helpers across the three retained semantic-convention versions and adds a semconvkit AST regression check so generated metric helpers must return after their no-attribute fast path.

Validation:

  • go test ./semconv/v1.32.0/...
  • go test ./semconv/v1.33.0/...
  • go test ./semconv/v1.34.0/...
  • go test ./semconvkit from internal/tools
  • make precommit completed generation, lint, README, and module checks, then stopped at the existing bridge/opentracing gRPC test with a server preface EOF; the changed semconv scopes pass.

@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.4%. Comparing base (1ad6638) to head (2a1366e).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #8849   +/-   ##
=====================================
  Coverage   88.4%   88.4%           
=====================================
  Files        331     331           
  Lines      21001   21001           
=====================================
  Hits       18572   18572           
  Misses      2429    2429           

see 2 files with indirect coverage changes

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

@jstar0

jstar0 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

The remaining check-links failure appears to be baseline-only, not caused by this PR.

  • Base: 736a14fcdca28b8cf5237e6b9b166ec6ed832bf7
  • Head: 37b816049b598997524f21847cf7e0322772a5c1
  • The six failing 403 URLs are unchanged in semconv/v1.32.0, v1.33.0, and v1.34.0 systemconv/metric.go.
  • The PR diff for those files only adds the no-attributes early return to Uptime.Record; it does not modify the referenced URLs.
  • The base branch's preceding check-links runs also reported unrelated 403 failures, confirming the checker is sensitive to existing external link availability.

All code-relevant checks for this head are passing. No unrelated generated-document link edits are proposed in this PR.

@MrAlias MrAlias left a comment

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.

The production change is focused, but the regression coverage needs to establish the one-call behavior before this is ready.

Comment thread internal/tools/semconvkit/main_test.go Outdated
@jstar0

jstar0 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the requested regression coverage in 5e0020a35.

  • Added a counting-instrument regression for semconv/v1.34.0/goconv.ScheduleDuration.Record without attributes; one helper call now must produce exactly one underlying Record call.
  • Tightened the generator test to enumerate all 60 affected Record methods across v1.32.0-v1.34.0 and require exactly one empty-attribute fast path for each.

Verification:

  • go test ./semconvkit from internal/tools: passed.
  • go test -race ./semconv/v1.34.0/goconv: passed.
  • make precommit reached the repository test phase, but is currently blocked by bridge/opentracing/TestBridgeTracer_ExtractAndInject_gRPC, which fails with error reading server preface: EOF in both race and non-race runs. No production code is changed by this follow-up.

@jstar0

jstar0 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the CHANGELOG.md conflict with current main via signed merge commit 2a1366e6e. The #8849 entry is kept under [Unreleased] > Fixed (the released sections are unchanged) and ./verify_released_changelog.sh main passes. No implementation files changed; the branch is mergeable again.

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.

Semconv v1.32.0-v1.34.0 metric helpers record twice without attributes

2 participants