-
Notifications
You must be signed in to change notification settings - Fork 600
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
Stripe integration Causes Cardinality Explosion #2709
Comments
Hi @jsneedles. Thanks very much for reporting this. The same issue has been reported in #2654, with a proposal that the agent replace all unique Stripe id values with an asterisk to dramatically reduce the number of unique names. Your proposal to strip away the ids entirely in the metric naming seems very similar. In the most recent comment left on #2654, @kford-newrelic explained our plan to treat this as a feature request as a opposed to a bug. But running up against New Relic's default DENY_NEW_METRICS rule and not being able to report anything but the first n unique Stripe metrics sounds like it would make for a pretty bad experience. I will freshly bring the matter back to the team's attention. In the meantime, the agent's on-by-default instrumentation of Stripe can be disabled in the instrumentation.stripe: disabled |
Thanks @fallwith I appreciate the context, and apologize as a quick search for I would definitely call this a bug since it is indeed not following the suggestions as outlined by stripe & is enabled by default. thanks for the consideration! |
Hi @jsneedles. We've put together #2716 to address this issue. So given a path of "/v1/payment_methods/pm_8675309", the metric name will now be "Stripe/v1/payment_methods/get". If you have any feedback for the PR, please let us know. The fix will go out in the next agent release, but you are welcome to test the change by pointing at the gem 'newrelic_rpm', github: 'newrelic/newrelic-ruby-agent', branch: 'stripefix' Thanks for reporting this and getting the ball rolling on it again! |
Awesome news @fallwith, thanks so much. We went ahead and disabled the integration and got our DENY removed ... but will definitely remember to give this a test with the next release. |
Hi @jsneedles. |
Hi,
I noticed when using the stripe integration (added in #2180 ) that metric cardinality explodes with each outgoing http call being traced as separate metrics.
Description
Specifically we're seeing metrics like
Stripe/v1/customers/cus_XXXXXXXXXXX/get
andStripe/v1/payment_methods/pm_XXXXXXXXX/get
etc...It caused a metric cardinality error and is preventing us from collecting any new metrics because of an auto-normalization.
Expected Behavior
We'd expect a metric name like
Stripe/v1/payment_methods/get
Your Environment
Additional context
Stripe's example instrumentation code shows the
path_parts
being split and dropping the middle (high cardinality) item.I believe this is what's missing.
For Maintainers Only or Hero Triaging this bug
Suggested Priority (P1,P2,P3,P4,P5):
Suggested T-Shirt size (S, M, L, XL, Unknown):
The text was updated successfully, but these errors were encountered: