Skip to content

reporter: simplify TraceReporter interface#405

Merged
christos68k merged 6 commits intomainfrom
reporter-TraceReporter
Mar 28, 2025
Merged

reporter: simplify TraceReporter interface#405
christos68k merged 6 commits intomainfrom
reporter-TraceReporter

Conversation

@florianl
Copy link
Copy Markdown
Member

This proposed change implements #384 (comment) as a first step of #384.

This proposed change implements #384 (comment) as a first step of #384.

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@florianl florianl requested review from a team as code owners March 17, 2025 12:54
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Comment thread tracehandler/tracehandler.go
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Comment on lines +109 to +110
// Wait to make sure the purge routine did start.
wg.Wait()
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.

Does it make sense to wait here? It doesn't harm if the go routine starts after return.

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.

The waitgroup pattern is just used to make sure the Go routine starts. This pattern is used also in other places in the code base. Waiting here should not increase the start up time in a noticeable way. But as we can not control the Go scheduler, we make sure this way, the clean up/purge routine is working properly and scheduled.

Copy link
Copy Markdown
Member

@christos68k christos68k left a comment

Choose a reason for hiding this comment

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

I'll do another pass and test a bit.

Comment thread tracehandler/tracehandler.go Outdated
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Comment thread tracehandler/tracehandler.go Outdated
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@christos68k christos68k merged commit 12c0fab into main Mar 28, 2025
@christos68k christos68k deleted the reporter-TraceReporter branch March 28, 2025 19:21
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request Mar 31, 2025
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request Apr 1, 2025
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request Apr 1, 2025
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request Apr 1, 2025
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request Apr 16, 2025
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request Apr 18, 2025
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request Apr 30, 2025
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request May 5, 2025
nsavoire added a commit to DataDog/dd-otel-host-profiler that referenced this pull request May 5, 2025
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.

3 participants