-
Notifications
You must be signed in to change notification settings - Fork 1.5k
SQLServer Extended Event Handlers #20229
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
Conversation
550ae52
to
19a6803
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, really appreciate the thorough commenting. A few questions/comments then LGTM
filtered_events = [] | ||
try: | ||
# Convert string to bytes for lxml | ||
xml_stream = BytesIO(xml_data.encode('utf-8')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the data returned always utf-8
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the raw data is taken at raw_xml = str(row[0])
, which is already getting a properly decoded string. I just choose an encoding here to convert it to bytes for lxml parsing. Just in case though, I can add back up logic to encode in utf-16 if we hit encoding errors.
Datadog Summary✅ Code Quality ✅ Code Security ❌ Dependencies Was this helpful? Give us feedback! |
What does this PR do?
Implements the SQLServer Extended Event Handlers. This enables deobfuscation and query error visibility. This is a beefy PR so I will describe at a high level each component. See the RFC here
Configuration
XE Handler
Events emitted
Currently collects three types of query completion events, emitted as dbm_type=query_completion:
Collects two types of error events, emitted as dbm_type=query_error:
Emits RQT (Raw Query Text) events when collect_raw_query_statement.enabled is true:
Testing
Motivation
Get query error and deobfuscated query visibility for sqlserver. This is a targeted feature for Rockstar, but greatly strengthens DBM's sqlserver offering.
Review checklist (to be filled by reviewers)
qa/skip-qa
label if the PR doesn't need to be tested during QA.backport/<branch-name>
label to the PR and it will automatically open a backport PR once this one is merged