Skip to content

Semconv metric helper caller-slice mutation fix#7993

Merged
pellared merged 2 commits intoopen-telemetry:mainfrom
MrAlias:semconv-metric-attr
Mar 4, 2026
Merged

Semconv metric helper caller-slice mutation fix#7993
pellared merged 2 commits intoopen-telemetry:mainfrom
MrAlias:semconv-metric-attr

Conversation

@MrAlias
Copy link
Copy Markdown
Contributor

@MrAlias MrAlias commented Mar 3, 2026

This PR addresses issue #7987 (issue (1)): generated semconv metric helpers should not risk mutating caller-provided attrs backing arrays when required attributes are appended.

Update the generator logic to merge required-attribute using capacity clamping before append:

append(attrs[:len(attrs):len(attrs)], requiredAttrs...)

This guarantees append allocates a new backing array for the merged slice instead of writing into caller memory when there are attributes appended. It makes not allocations in the case attributes are not appended.

@MrAlias MrAlias added this to the v1.42.0 milestone Mar 3, 2026
@MrAlias MrAlias added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Mar 3, 2026
@MrAlias MrAlias marked this pull request as ready for review March 3, 2026 17:25
@MrAlias MrAlias requested a review from XSAM as a code owner March 3, 2026 17:25
Copilot AI review requested due to automatic review settings March 3, 2026 17:25
@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 (8c91c83) to head (9f707e8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7993     +/-   ##
=======================================
- Coverage   81.6%   81.6%   -0.1%     
=======================================
  Files        304     304             
  Lines      23385   23385             
=======================================
- Hits       19093   19089      -4     
- Misses      3905    3907      +2     
- Partials     387     389      +2     

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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Go semconv metric helper generation to avoid mutating caller-provided attrs backing arrays when required semantic attributes are appended, and includes regenerated v1.40.0 metric helper outputs.

Changes:

  • Update the Weaver Go template to cap attrs capacity before appending required attributes (attrs[:len(attrs):len(attrs)]).
  • Regenerate affected semconv/v1.40.0/*conv/metric.go files to apply the new append behavior.
  • Replace direct append(attrs, ...) patterns with the capacity-clamped variant across the regenerated outputs.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
semconv/templates/registry/go/instrument.j2 Adjusts generated helper attribute merging to avoid mutating caller slices.
semconv/v1.40.0/azureconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/cicdconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/dbconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/dnsconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/genaiconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/httpconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/hwconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/k8sconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/mcpconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/messagingconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/openshiftconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/rpcconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.
semconv/v1.40.0/vcsconv/metric.go Regenerated to use capacity-clamped attrs before appending required attrs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pellared pellared merged commit b2b3250 into open-telemetry:main Mar 4, 2026
31 of 32 checks passed
@MrAlias MrAlias deleted the semconv-metric-attr branch March 4, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants