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

[connector/datadog] Support obfuscating sql queries in APM stats #35401

Closed
wants to merge 2 commits into from

Conversation

songy23
Copy link
Member

@songy23 songy23 commented Sep 24, 2024

Description:
Support obfuscating sql queries in APM stats in DD connector
This simulates the same in trace agent: https://github.com/DataDog/datadog-agent/blob/7642cf1aa659f82744038602044a8a00aa1a0dfb/pkg/trace/agent/obfuscate.go#L109

@songy23 songy23 requested review from mx-psi and a team as code owners September 24, 2024 15:28
Copy link
Contributor

github-actions bot commented Oct 9, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 9, 2024
@songy23 songy23 added on hold This is blocked by another PR/issue never stale Issues marked with this label will be never staled and automatically removed and removed Stale labels Oct 9, 2024
@songy23
Copy link
Member Author

songy23 commented Dec 9, 2024

cc @jackgopack4 for context

@@ -158,3 +176,20 @@ func (c *traceToMetricConnectorNative) run() {
}
}
}

// fork of https://github.com/DataDog/datadog-agent/blob/7642cf1aa659f82744038602044a8a00aa1a0dfb/pkg/trace/agent/obfuscate.go#L109
Copy link
Contributor

Choose a reason for hiding this comment

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

is this sustainable long-term? Is it possible we could add this as a function to pkg/obfuscate instead so that it doesn't get forgotten when obfuscate folks make changes to their obfuscateStatsGroup calculations?

Copy link
Contributor

Choose a reason for hiding this comment

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

we are already having to import pkg/obfuscate; I feel like we could add a function there that takes the c.obfuscator as a parameter instead

Copy link
Contributor

Choose a reason for hiding this comment

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

submitted a PR to modify the signature of this function in trace agent: DataDog/datadog-agent#32064

@songy23
Copy link
Member Author

songy23 commented Jan 8, 2025

closing in favor of DataDog/datadog-agent#32750

@songy23 songy23 closed this Jan 8, 2025
songy23 pushed a commit that referenced this pull request Jan 24, 2025
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Support obfuscating SQL and Redis queries in APM stats in Datadog
Connector.

replace statement in otelcontribcol was necessary due to failing build
in pipeline. It doesn't appear that any modules use any affected
functions, but building otelcontribcol with ocb failed due to an API
change. Also opened an issue to bump the `receiver/awscontainerinsight`
dependency so that this replace can be removed in the future: #37486

<!-- Issue number (e.g. #1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Replaces #35401

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Added TestObfuscate in connector_native_test.go

<!--Describe the documentation added.-->
#### Documentation
added release note and link to semantics on Datadog vendor website
<!--Please delete paragraphs that you did not use before submitting.-->
chengchuanpeng pushed a commit to chengchuanpeng/opentelemetry-collector-contrib that referenced this pull request Jan 26, 2025
…emetry#37457)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Support obfuscating SQL and Redis queries in APM stats in Datadog
Connector.

replace statement in otelcontribcol was necessary due to failing build
in pipeline. It doesn't appear that any modules use any affected
functions, but building otelcontribcol with ocb failed due to an API
change. Also opened an issue to bump the `receiver/awscontainerinsight`
dependency so that this replace can be removed in the future: open-telemetry#37486

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Replaces open-telemetry#35401

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Added TestObfuscate in connector_native_test.go

<!--Describe the documentation added.-->
#### Documentation
added release note and link to semantics on Datadog vendor website
<!--Please delete paragraphs that you did not use before submitting.-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connector/datadog never stale Issues marked with this label will be never staled and automatically removed on hold This is blocked by another PR/issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants