Skip to content

fix(ddtrace/tracer): prevent applying rate to finished spans and add test coverage for this finishing code path (#3474 backport)#3476

Merged
darccio merged 1 commit into
v1-maintenancefrom
dario.castane/finished-check-applyrate-v1
Apr 30, 2025
Merged

fix(ddtrace/tracer): prevent applying rate to finished spans and add test coverage for this finishing code path (#3474 backport)#3476
darccio merged 1 commit into
v1-maintenancefrom
dario.castane/finished-check-applyrate-v1

Conversation

@darccio

@darccio darccio commented Apr 30, 2025

Copy link
Copy Markdown
Member

What does this PR do?

Backports #3474.

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running golangci-lint run locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@darccio
darccio requested a review from a team as a code owner April 30, 2025 12:04
felixge
felixge reviewed Apr 30, 2025
View reviewed changes
Comment thread ddtrace/tracer/span.go Outdated

// AddEvent attaches a new event to the current span.
func (s *span) AddEvent(name string, opts ...ddtrace.SpanEventOption) {
func (s *Span) AddEvent(name string, opts ...SpanEventOption) {

@felixge felixge Apr 30, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this will compile 🙈

@darccio darccio Apr 30, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ouch.

@darccio darccio Apr 30, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed.

@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Apr 30, 2025 •
edited
Loading

Copy link
Copy Markdown

Datadog Report

Branch report: dario.castane/finished-check-applyrate-v1
Commit report: 691d6c2
Test service: dd-trace-go

✅ 0 Failed, 5510 Passed, 72 Skipped, 11m 14.1s Total Time

@darccio
darccio force-pushed the dario.castane/finished-check-applyrate-v1 branch from befd35d to 6cdd070 Compare April 30, 2025 12:09
@darccio
darccio merged commit ee87977 into v1-maintenance Apr 30, 2025
@darccio
darccio deleted the dario.castane/finished-check-applyrate-v1 branch April 30, 2025 12:30
darccio added a commit that referenced this pull request Apr 30, 2025
@darccio
fix(ddtrace/tracer): prevent applying rate to finished spans and add …
4afdbe5 
…test coverage for this finishing code path (#3474 backport) (#3476)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixge felixge felixge approved these changes
@genesor genesor genesor approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@darccio @felixge @genesor