-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add DBM auto_explain configuration #34146
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
Preview links (active after the
|
rtrieu
left a comment
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.
some minor feedback to comply with our style guide
|
|
||
| ### Collecting plans with `auto_explain` (optional) | ||
|
|
||
| By default, the agent will only gather `EXPLAIN` plans for a sampling of in-flight queries. These plans are of a more general nature, especially when application code uses prepared statements. |
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.
WDYT about linking to postgres EXPLAIN docs?
|
|
||
| By default, the agent will only gather `EXPLAIN` plans for a sampling of in-flight queries. These plans are of a more general nature, especially when application code uses prepared statements. | ||
|
|
||
| To collect full `EXPLAIN ANALYZE` plans taken from all queries, you will need to use `auto_explain`, a first-party extension bundled with PostgreSQL available in all major providers. _Logging collection is a prerequisite to `auto_explain` collection_, so be sure to enable it before continuing. |
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.
Likewise wdyt about linking to PG docs for autoexplain?
https://www.postgresql.org/docs/current/auto-explain.html
|
|
||
| 2. Change the `log_line_prefix` to enable richer event correlation | ||
| ```conf | ||
| log_line_prefix = '%m:%r:%u@%d:[%p]:%l:%e:%s:%v:%x:%c:%q%a:' # this pattern is required to ingest auto_explain plans |
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.
The "this is required" portion is truncated by the code snippet box. I'd pull it out into the step 2 text ( ...richer event correlation. This pattern is required to ...)
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.
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.
Either pattern works; both contain the important things for correlation.
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.
The "this is required" portion is truncated by the code snippet box
Yeah, I modeled that comment after the earlier log config line, which also had a trailing comment kind of far off to the right.
I've moved it into the step two text.
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.
It also conflicts with the "Configure logging" section above:
I wasn't sure whether to address that. The logging section config is sufficient for sending and tagging PostgreSQL logs, but an explain plan needs more fields than are present there.
The auto_explain log config also is sufficient for sending and tagging PostgreSQL logs, so it's a functional superset. I didn't know whether to address this in text, or just implicitly imply this config supercedes the section above…
| To collect full `EXPLAIN ANALYZE` plans taken from all queries, you will need to use `auto_explain`, a first-party extension bundled with PostgreSQL available in all major providers. _Logging collection is a prerequisite to `auto_explain` collection_, so be sure to enable it before continuing. | ||
|
|
||
| <div class="alert alert-danger"> | ||
| <strong>Important:</strong> <code>auto_explain</code> produces logs lines that may contain sensitive information from your application, similar to the raw values that appear in non-obfuscated SQL. You can use the FIXME permission to control who can see the resulting plans, but the log lines themselves <i>will</i> be visible to all users within your Datadog org. Using <a href="//logs/guide/logs-rbac">RBAC for Logs</a> is one way to ensure these logs are only visible to the right users. |
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.
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.
That's weird. @rtrieu how do I make a link to another docs page from within an HTML block (can't use markdown to generate the link)?
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.
Oh, dur, I used a double slash by accident. Fixed.
Mostly linter stuff. Co-authored-by: Rosa Trieu <107086888+rtrieu@users.noreply.github.com>
Co-authored-by: Rosa Trieu <107086888+rtrieu@users.noreply.github.com>
|
This will include docs for RDS as well right? |

What does this PR do? What is the motivation?
Adds a section about configuring
auto_explainto the DBM PostgreSQL documentation.Merge instructions
Going to use this preview to discuss language with team; after which I will mark this ready for merge, but not yet.
Merge readiness:
Additional notes
This feature is in preview.