ddtrace/tracer: update log msg to accurately count dropped traces by hannahkm · Pull Request #2845 · DataDog/dd-trace-go

hannahkm · 2024-09-06T20:33:38Z

What does this PR do?

Lower the severity of the payload queue log to debug.
Introduce a new value in the tracer, totalTracesDropped to keep track of the number of dropped traces.
Log the total number of dropped traces, if they exist, every one second.

Motivation

This PR addresses an issue raised in APMAPI-225. Log messages produced when dropping chunks were too spammy (sometimes occurring > 1,000 times per second) and relayed inaccurate information.

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.
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!

pr-commenter · 2024-09-06T21:28:49Z

Benchmarks

Benchmark execution time: 2024-09-24 14:26:09

Comparing candidate commit 214c1bc in PR branch hannahs/log-dropped-traces with baseline commit 7699f9e in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 58 metrics, 1 unstable metrics.

mtoffl01

I don't want to block this merge, so I am approving as it looks all clear, but in my opinion and from a troubleshooting perspective, I don't love the approach of "Every X seconds, check how many traces were dropped in the previous interval, and log about it." Logging about the dropped trace at the moment it is dropped is better, but you'd have to go back to using mutexes (or related) to achieve that.

dianashevchenko · 2024-09-18T20:22:22Z

@mtoffl01 we got two requests internally to change the logging because of how spammy it was. Previously, it was indeed emitting a log each time the trace was dropped. So, my suggestion would be to make the interval configurable and smaller, for example 1 second, but avoid logging at each trace drop. What do you think? I can also point you to the original slack threads.

hannahkm · 2024-09-24T13:52:19Z

/merge

dd-devflow · 2024-09-24T13:52:24Z

❌ MergeQueue

You are not allowed to use the merge queue towards main.

If you need support, contact us on Slack #devflow with those details!

hannahkm · 2024-09-24T13:56:59Z

/merge

dd-devflow · 2024-09-24T13:57:11Z

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 0s.

Use /merge -c to cancel this operation!

dd-devflow · 2024-09-24T14:03:40Z

❌ MergeQueue: The checks failed on this merge request

Tests failed on this commit dfedcf5:

PR Unit and Integration Tests / test-contrib

What to do next?

Investigate the failures and when ready, re-add your pull request to the queue!
Any question, go check the FAQ.

If you need support, contact us on Slack #devflow with those details!

dianashevchenko · 2024-09-24T14:24:07Z

/merge --cancel

hannahkm · 2024-09-24T15:31:12Z

/merge

dd-devflow · 2024-09-24T15:31:21Z

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 0s.

Use /merge -c to cancel this operation!

dd-devflow · 2024-09-24T15:56:02Z

🚨 MergeQueue: This merge request is in error

mergequeue build completed successfully, but the github api returned an error while merging the pr.
It's probably because:

repository configuration requires approval by code owners, but no code owner approved this PR
target branch of PR is restricted to only allow up-to-date branches, but the pr is now outdated

Details

Error: PUT https://api.github.com/repos/DataDog/dd-trace-go/pulls/2845/merge: 405 5 of 5 required status checks are expected. []

FullStacktrace:
activity error (type: github.GithubService_MergePullRequest, scheduledEventID: 41, startedEventID: 42, identity: 1@github-worker-b9c46c97c-gl2p7@): PUT https://api.github.com/repos/DataDog/dd-trace-go/pulls/2845/merge: 405 5 of 5 required status checks are expected. [] (type: ErrorResponse, retryable: true)

If you need support, contact us on Slack #devflow with those details!

hannahkm · 2024-09-24T16:05:56Z

/merge

dd-devflow · 2024-09-24T16:06:08Z

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 0s.

Use /merge -c to cancel this operation!

hannahkm added 3 commits September 6, 2024 16:23

update log msg to accurately count dropped traces

48d15bc

resolve null pointer exception

1c62c41

resolve data race in profiler

516e929

hannahkm and others added 5 commits September 9, 2024 09:42

resolve data race w mux

dd1a10a

code clean

cc17a6d

replace all atomic with mutex

4c97b59

Merge branch 'main' into hannahs/log-dropped-traces

32ebb2d

Merge branch 'main' into hannahs/log-dropped-traces

070866a

hannahkm marked this pull request as ready for review September 9, 2024 21:09

hannahkm requested a review from a team as a code owner September 9, 2024 21:09

mtoffl01 reviewed Sep 10, 2024

View reviewed changes

Comment thread ddtrace/tracer/tracer.go Outdated

mtoffl01 reviewed Sep 10, 2024

View reviewed changes

Comment thread ddtrace/tracer/tracer.go Outdated

hannahkm and others added 7 commits September 10, 2024 15:20

clarify finishChunk test and remove metrics test

d4ec88a

replace mux w ticker (tests may fail)

3442a22

revert to mux using rwmux

a0e93c9

Merge branch 'main' into hannahs/log-dropped-traces

17a9333

return to using ticker, (hopefully) without data race

673b4b0

lint: remove unused var

3e55527

use of atomic to ensure no races

5bc24a1

hannahkm marked this pull request as draft September 17, 2024 14:36

hannahkm marked this pull request as ready for review September 17, 2024 15:07

Merge branch 'main' into hannahs/log-dropped-traces

1f58837

mtoffl01 approved these changes Sep 17, 2024

View reviewed changes

hannahkm and others added 2 commits September 20, 2024 10:52

reduce time to log to 1 sec

1cf18a9

Merge branch 'main' into hannahs/log-dropped-traces

214c1bc

dd-devflow Bot added mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: rejected and removed mergequeue-status: queued mergequeue-status: in_progress labels Sep 24, 2024

dd-devflow Bot added mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: error and removed mergequeue-status: rejected mergequeue-status: queued mergequeue-status: in_progress labels Sep 24, 2024

Merge branch 'main' into hannahs/log-dropped-traces

8e3c972

dd-devflow Bot added mergequeue-status: queued mergequeue-status: in_progress and removed mergequeue-status: error mergequeue-status: queued labels Sep 24, 2024

dd-mergequeue Bot merged commit eef52d3 into main Sep 24, 2024

dd-devflow Bot added mergequeue-status: done and removed mergequeue-status: in_progress labels Sep 24, 2024

dd-mergequeue Bot deleted the hannahs/log-dropped-traces branch September 24, 2024 16:30

Uh oh!

Conversation

hannahkm commented Sep 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Reviewer's Checklist

Uh oh!

pr-commenter Bot commented Sep 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

Uh oh!

Uh oh!

mtoffl01 left a comment

Choose a reason for hiding this comment

Uh oh!

dianashevchenko commented Sep 18, 2024

Uh oh!

hannahkm commented Sep 24, 2024

Uh oh!

dd-devflow Bot commented Sep 24, 2024

Uh oh!

hannahkm commented Sep 24, 2024

Uh oh!

dd-devflow Bot commented Sep 24, 2024

Uh oh!

dd-devflow Bot commented Sep 24, 2024

What to do next?

Uh oh!

dianashevchenko commented Sep 24, 2024

Uh oh!

hannahkm commented Sep 24, 2024

Uh oh!

dd-devflow Bot commented Sep 24, 2024

Uh oh!

dd-devflow Bot commented Sep 24, 2024

Uh oh!

hannahkm commented Sep 24, 2024

Uh oh!

dd-devflow Bot commented Sep 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hannahkm commented Sep 6, 2024 •

edited

Loading

pr-commenter Bot commented Sep 6, 2024 •

edited

Loading