Drop the official Datadog client #144
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This drops our dependency on `@datadog/datadog-api-client` in favor of directly calling the API via HTTP. It requires a little more code on our side, but gets us a bit more flexibility and drops a 15 MB (!) dependency that we barely use. Fixes #132.
Not strictly required for this PR, but these changes help make tests a safer and more consistent regardless of what env vars you have set.
Mr0grog
commented
Jan 2, 2025
| get () { return false; }, | ||
| set () {}, | ||
| }); | ||
| class HttpApi { |
There was a problem hiding this comment.
Probably want to either merge this into DatadogReporter or pull some more logic from it (e.g. base URL stuff) down into this class.
|
Thanks for this, I appreciate the thought behind this! |
|
@jonathanstiansen do need support for Node.js < 12? Would you be interested in testing a pre-release version of this? |
|
Nope!
…On Sun, Apr 13, 2025 at 10:59 AM Rob Brackett ***@***.***> wrote:
@jonathanstiansen <https://github.com/jonathanstiansen> do need support
for Node.js < 12?
Would you be interested in testing a pre-release version of this?
—
Reply to this email directly, view it on GitHub
<#144 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAR7T2EL6DJESUAO2YU2YL32ZKJXJAVCNFSM6AAAAABUQWABPCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMBQGAZTENRUGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
*Mr0grog* left a comment (dbader/node-datadog-metrics#144)
<#144 (comment)>
@jonathanstiansen <https://github.com/jonathanstiansen> do need support
for Node.js < 12?
Would you be interested in testing a pre-release version of this?
—
Reply to this email directly, view it on GitHub
<#144 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAR7T2EL6DJESUAO2YU2YL32ZKJXJAVCNFSM6AAAAABUQWABPCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMBQGAZTENRUGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first pass, and is a bit speculative. This PR drops the official Datadog client as a runtime dependency since it is huge (15 MB!) and imposes a lot of runtime costs and some logging confusion (e.g. #133).
The implementation here basically swaps out
@datadog/datadog-api-clientforcross-fetch, which is what@datadog/datadog-api-clientwas using, so the potential issues should be pretty minimal here.I was hoping to fix the punycode deprecation warning, but was not able to do so because the dependencies causing the issue require newer versions of Node.js than we currently do (Node.js 12.0). We can upgrade
cross-fetch(requires Node.js 14.0) or switch directly tonode-fetch(requires Node.js 12.20, but cuts off the tacit-but-not-guaranteed-compatibility we have with React Native and with Browsers). Either one of these is probably OK (Node.js maintenance only extends back to v18 these days anyway, and our biggest dependee, datadog-ci, only requires Node.js 14). Something to think through before landing this.