Conversation
| "devDependencies": { | ||
| "@aws-sdk/client-kms": "^3.366.0", | ||
| "@aws-sdk/client-secrets-manager": "^3.721.0", | ||
| "@datadog/pprof": "*", |
There was a problem hiding this comment.
Can we not just define it as regular dependency? Similar with dd-trace?
There was a problem hiding this comment.
This is so that it resolves to the version transitively installed by dd-trace.
There was a problem hiding this comment.
And for dd-trace, it shouldn't be a regular dependency because if you don't need profiling there is no reason to install it. Ideally we'd have a better modularized project that ships each package individually and then have top level packages that only include what they need, but right now optional dependencies seem like the easiest way to achieve something similar.
There was a problem hiding this comment.
Thinking about it: this would change the behavior compared to before. We pinned pprof and profiling does not want to get auto updates. So if I am not mistaken, we could end up installing a newer version than the one in dd-trace.
There was a problem hiding this comment.
This should use the pinned version no? The package manager will resolve, realize that the pinned version fulfills both ranges, and use that for both.
There was a problem hiding this comment.
I am uncertain how this install works here. It seems to be copied and it does not end up as regular dependency, so you are probably correct.
What does this PR do?
Fix missing
@datadog/pprofdependency.Motivation
Removing optional dependencies broke profiling since
@datadog/pprofis now an optional dependency.Testing Guidelines
Good question 😅 This should be caught by CI but it doesn't seem profiling is tested in CI.
Additional Notes
Types of Changes
Check all that apply