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.

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

Add flush method to tracer #4805

Closed
wants to merge 23 commits into from
Closed

Add flush method to tracer #4805

wants to merge 23 commits into from

Conversation

wu-json
Copy link

@wu-json wu-json commented Oct 20, 2024

What does this PR do?

Adds a .flush() method to the tracer, which allows users to manually flush collected spans to the exporter agent. This is useful for ensuring spans aren't lost when force exiting a process with process.exit.

Motivation

My team recently migrated our Node.js crons from NewRelic to Datadog. In NewRelic land, we had access to newrelic.shutdown({ collectPendingData: true }), which allowed us to send all metrics and traces to NewRelic before killing the process with process.exit (we do not exit gracefully in our crons for reasons I will not dive into here).

In Datadog land however, we found that such a method did not exist, which means we were not getting traces when crons completed. The only workaround is to set a delay before exiting the process, which is hacky and not guaranteed to work.

There are two existing threads with conversations requesting this feature as well.

The other PR that attempted to add this functionality was abandoned so I'm taking a stab at picking up where they left off.

Checklist

Additional Notes

I really want this feature - happy to work with maintainers on making whatever edits we need to get this in.

@wu-json wu-json changed the title Add tracer.flush() Add flush method to tracer Oct 20, 2024
@wu-json wu-json marked this pull request as ready for review October 20, 2024 04:34
@wu-json wu-json requested a review from a team as a code owner October 20, 2024 04:34
@juan-fernandez juan-fernandez mentioned this pull request Oct 21, 2024
@simon-id
Copy link
Member

FYI There is no point rebasing this PR on master anymore, as Juan recreated it to run the CI

@juan-fernandez
Copy link
Collaborator

hey @wu-json thanks for the contribution. We've discussed this internally and while we like it, we're afraid to deviate from the public API other datadog libraries have. We're thinking of a way forward. Stay tuned 😄

@wu-json
Copy link
Author

wu-json commented Oct 24, 2024

hey @wu-json thanks for the contribution. We've discussed this internally and while we like it, we're afraid to deviate from the public API other datadog libraries have. We're thinking of a way forward. Stay tuned 😄

@juan-fernandez Ah - understood. Let me know if I can be of use here after the direction is sorted out internally. Would be happy to help with the solution.

@wu-json wu-json closed this Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants