Skip to content
This repository was archived by the owner on Aug 11, 2025. It is now read-only.

fix: add Close method#1

Merged
micaelmalta merged 1 commit into
mainfrom
mickey/fix_flush
May 14, 2024
Merged

fix: add Close method#1
micaelmalta merged 1 commit into
mainfrom
mickey/fix_flush

Conversation

@micaelmalta

@micaelmalta micaelmalta commented May 13, 2024

Copy link
Copy Markdown
Collaborator

This enable flushing correctly the metrics

By default, metrics are buffered and sent to Datadog every 100ms

If we are for example shutting down an app, some metrics might be lost

By Closing it properly, we ensure every metrics will be actually sent

This enable flushing correctly the metrics
@jjacobskind

Copy link
Copy Markdown

Without having much context here, would it make more sense to throw this PR up on the Shopify repo?

Once we fork it, we own it. If we get them to merge this change, they benefit and we don't have to maintain it

@jjacobskind jjacobskind left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for sharing the context that @mtwstudios threw up a PR a while back that never got merged. I think it's worth reminding the Shopify folks about the PR, since it's been two years

Approving though, in case they don't address it

@micaelmalta micaelmalta merged commit ede1a99 into main May 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants