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

Remove query string from resource.name by default #2631

Merged
merged 1 commit into from
Dec 22, 2022

Conversation

Qard
Copy link
Collaborator

@Qard Qard commented Dec 21, 2022

What does this PR do?

This removes the query string from resource.name by default, allowing it to be restored with an additional config.

Motivation

Queries may contain private data so should not be included in resource.name by default, but with the queryInResourceName configuration it can be optionally restored.

Plugin Checklist

Fixes #2536
Fixes #2622

Queries may contain private data so should not be included in resource.name by default,
but with the queryInResourceName configuration it can be optionally enabled.
@Qard Qard added the bug Something isn't working label Dec 21, 2022
@Qard Qard requested a review from a team as a code owner December 21, 2022 08:15
@codecov
Copy link

codecov bot commented Dec 21, 2022

Codecov Report

Merging #2631 (e0f89aa) into master (483861c) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2631   +/-   ##
=======================================
  Coverage   89.00%   89.00%           
=======================================
  Files         298      298           
  Lines       10381    10381           
  Branches       33       33           
=======================================
  Hits         9240     9240           
  Misses       1141     1141           
Impacted Files Coverage Δ
packages/datadog-plugin-mongodb-core/src/index.js 97.50% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pr-commenter
Copy link

pr-commenter bot commented Dec 21, 2022

Benchmarks

Comparing candidate commit e0f89aa in PR branch fix-mongo-query-resource-name-cardinality with baseline commit 483861c in branch master.

Found 0 performance improvements and 1 performance regressions! Performance is the same for 965 cases.

@Qard Qard merged commit bc4d59e into master Dec 22, 2022
@Qard Qard deleted the fix-mongo-query-resource-name-cardinality branch December 22, 2022 16:56
Qard added a commit that referenced this pull request Dec 22, 2022
Queries may contain private data so should not be included in resource.name by default,
but with the queryInResourceName configuration it can be optionally enabled.
Qard added a commit that referenced this pull request Dec 22, 2022
Queries may contain private data so should not be included in resource.name by default,
but with the queryInResourceName configuration it can be optionally enabled.
Qard added a commit that referenced this pull request Dec 23, 2022
Queries may contain private data so should not be included in resource.name by default,
but with the queryInResourceName configuration it can be optionally enabled.
juan-fernandez pushed a commit that referenced this pull request Jan 3, 2023
Queries may contain private data so should not be included in resource.name by default,
but with the queryInResourceName configuration it can be optionally enabled.
juan-fernandez pushed a commit that referenced this pull request Jan 4, 2023
Queries may contain private data so should not be included in resource.name by default,
but with the queryInResourceName configuration it can be optionally enabled.
@pinko-fowle
Copy link

pinko-fowle commented Jan 5, 2023

@Qard So, in the Resource menu in tracing, we're going to lose all this route information, unless we go explicitly change the queryInResourceName option?

This would just be GET, POST, DELETE here?
Screen Shot 2023-01-05 at 4 32 46 PM

And in the trace view as well? Woof. There should be an option for this, but I think we're going way overboard in catering to the rare folk who do have an issue. This greatly degrades the default behavior of the product, if I'm understanding this right.

@Qard
Copy link
Collaborator Author

Qard commented Jan 6, 2023

No, this does not impact HTTP route names, this is specifically for mongodb queries. Previously it produced span resource names something like find my.collection { some: 'query' }, now it will just be find my.collection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mongo statement should not be part of the resource name MongoDB no longer sanitizes resource_names
4 participants