Skip to content

pkg/trace/obfuscate: fix bug where obfuscation fails for autovacuum sql text#9649

Merged
gbbr merged 1 commit intomainfrom
jmeunier/fix-autovacuum-sql
Nov 4, 2021
Merged

pkg/trace/obfuscate: fix bug where obfuscation fails for autovacuum sql text#9649
gbbr merged 1 commit intomainfrom
jmeunier/fix-autovacuum-sql

Conversation

@jmeunier28
Copy link
Copy Markdown
Contributor

@jmeunier28 jmeunier28 commented Oct 25, 2021

What does this PR do?

we are seeing query text like autovacuum: VACUUM ANALYZE fake.table fail obfuscation in the postgres integration check, and we want to be able to ingest things like this for DBM. It is failing due to the fact that it has a : followed by a space char. Because it has a :, the obfuscater assumes it is the beginning of a bind variable. Since the bindVar scanner code does not handle this case, obfuscation fails.

simply checking if the char in front of the : is a space will prevent this from happening

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Describe how to test your changes

Write here in detail how you have tested your changes
and instructions on how this should be tested in QA.

Describe or link instructions to set up environment
for testing, if the process requires more than just
running the agent on one of the supported platforms.

Checklist

  • A release note has been added or the changelog/no-changelog label has been applied.
  • The need-change/operator and need-change/helm labels has been applied if applicable.
  • The appropriate team/.. label has been applied, if known.
  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • The config template has been updated if applicable.

Note: Adding GitHub labels is only possible for contributors with write access.

Comment thread pkg/trace/obfuscate/sql_test.go Outdated
@jmeunier28 jmeunier28 added this to the 7.33.0 milestone Oct 29, 2021
@jmeunier28 jmeunier28 marked this pull request as ready for review November 2, 2021 15:04
@jmeunier28 jmeunier28 requested a review from a team as a code owner November 2, 2021 15:04
Comment thread pkg/trace/obfuscate/sql_tokenizer.go Outdated
Comment thread pkg/trace/obfuscate/sql_test.go Outdated
Copy link
Copy Markdown

@dujuku dujuku left a comment

Choose a reason for hiding this comment

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

I agree with this direction, but one small comment about returning the correct token and not a bind var.

Copy link
Copy Markdown

@dujuku dujuku left a comment

Choose a reason for hiding this comment

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

👍

@gbbr gbbr changed the title Fix bug where obfuscation fails for autovacuum sql text pkg/trace/obfuscate: fix bug where obfuscation fails for autovacuum sql text Nov 3, 2021
Comment thread pkg/trace/obfuscate/sql_tokenizer.go
Comment thread pkg/trace/obfuscate/sql_tokenizer.go
@gbbr gbbr added team/agent-apm trace-agent and removed changelog/no-changelog No changelog entry needed labels Nov 3, 2021
@gbbr
Copy link
Copy Markdown
Contributor

gbbr commented Nov 3, 2021

You should add a release note for this. Please make sure to prefix it with APM: . Relevant links:

@jmeunier28 jmeunier28 force-pushed the jmeunier/fix-autovacuum-sql branch 2 times, most recently from fec5440 to 4f211c4 Compare November 3, 2021 19:27
@jmeunier28 jmeunier28 force-pushed the jmeunier/fix-autovacuum-sql branch from 4f211c4 to 45d990b Compare November 3, 2021 19:29
@jmeunier28
Copy link
Copy Markdown
Contributor Author

You should add a release note for this. Please make sure to prefix it with APM: . Relevant links:
How to add a release note: https://github.com/DataDog/datadog-agent/blob/main/docs/dev/contributing.md#reno
Example release note: https://github.com/DataDog/datadog-agent/blob/main/releasenotes/notes/apm-credit-cards-obfuscation-918935ff76b3d897.yaml

thanks for review, @gbbr! look OK now?

Copy link
Copy Markdown
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!

@gbbr gbbr merged commit 3f7eac8 into main Nov 4, 2021
@gbbr gbbr deleted the jmeunier/fix-autovacuum-sql branch November 4, 2021 10:28
zandrewitte pushed a commit to StackVista/stackstate-agent that referenced this pull request Nov 17, 2022
…ql text (DataDog#9649)

For example, SQL text like `autovacuum: VACUUM ANALYZE fake.table` will no longer fail obfuscation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants