Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

parametric/apps/golang: Flush() doesn't flush stats #596

Closed
katiehockman opened this issue Oct 25, 2022 · 3 comments · Fixed by #794
Closed

parametric/apps/golang: Flush() doesn't flush stats #596

katiehockman opened this issue Oct 25, 2022 · 3 comments · Fixed by #794

Comments

@katiehockman
Copy link
Contributor

The FlushTraceStats function for the Go implementation should be able to call Flush(), but this function doesn't actually flush the trace stats. This causes problems for the some of the tests which are then forced to call Stop().

#576 will include some stopgaps, but we need to investigate if the Go Tracer should actually be flushing stats in Flush()

/cc @Kyle-Verhoog
@katiehockman katiehockman mentioned this issue Oct 25, 2022
Merged
3 tasks
katiehockman added a commit that referenced this issue Oct 26, 2022
@katiehockman
apps/golang: fix flush and enable span sampling tests
9bd062b 
Enable single span sampling tests for the Go Tracer after fixes in DataDog/dd-trace-go#1509.

Some bugs in the test setup needed to be fixed in order to accomplish this:

The Flush APIs needed to call Flush instead of Stop in the single span tests. For the tracestats tests, they needed to call Stop, so a new API endpoint was added to allow the tracestats to stop the tracer explicitly. See parametric/apps/golang: Flush() doesn't flush stats #596 for more details.
One of the tests was flaky at 1 second. Moving it to 2 appears to deflake this.
This PR updates the go.mod to point to DataDog/dd-trace-go#1509, which is now merged.
@katiehockman
Copy link
Contributor Author

/cc @DataDog/apm-go

@cbeauchesne
Copy link
Collaborator

@katiehockman can we close this issue ?

@katiehockman
Copy link
Contributor Author

Nope it's still a bug, albeit it could be argued that the bug is actually in the Go tracer, so I can file one there and link to this issue. Right now there is a temporary work (calling close when it should call flush for Go tests) around in the parametric tests that we need to remove once this is resolved, so it would be good to track this here to make sure we don't forget to remove it..

@katiehockman katiehockman mentioned this issue Oct 27, 2022
Closed
lievan added a commit that referenced this issue Jan 18, 2023
@lievan
test_library now calls flush() instead of stop() in tracestats tests …
818385b 
…after fix to #596
@lievan lievan mentioned this issue Jan 19, 2023
Merged
3 tasks
zacharycmontoya pushed a commit that referenced this issue Feb 17, 2023
@lievan @zacharycmontoya
test_library now calls flush() instead of stop() in tracestats tests …
b354411 
…after fix to #596
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

parametrics/apps/golang: call flush instead of stop in tracestats tests DataDog/system-tests
2 participants
@cbeauchesne @katiehockman