[DO NOT MERGE] Test fixture: editorial issues for dashboard review composer - #38672
[DO NOT MERGE] Test fixture: editorial issues for dashboard review composer#38672buraizu wants to merge 3 commits into
Conversation
Preview links (active after the
|
buraizu
left a comment
There was a problem hiding this comment.
- New section restates existing content and breaks the page's heading pattern
Every other heading on this page is phrased as a question, matching the page's FAQ format; this new heading is a title-case noun phrase, breaking both the question pattern and the sentence-case rule for headings. Its content — that custom-metric counts for percentile-enabled distributions are counted like other metric types — already appears earlier on the page in the "How does this new pipeline for Distributions affect me?" section, and the body is written entirely in passive voice.
Suggested fix: Remove this section since it doesn't add new information beyond what's already stated earlier on the page. If it's kept, rephrase the heading as a question in sentence case and rewrite the body in active voice.
- Bulk-configure bullet list is not parallel
The four list items mix grammatical forms (a declarative statement, an imperative instruction, another declarative statement, and a bare noun phrase), and are inconsistent in capitalization and end punctuation — only one item is capitalized and only one ends with a period.
Suggested fix: Confirm that your wildcard pattern matches only the intended metric names.
- test
|
[Automated message] This PR has merge conflicts created by the docs repo reorg project. Because this PR is marked as a work in progress, no attempt was made to auto-resolve the conflicts. When your PR is finished, you can queue your PR for an auto-fix by removing the This is an automated comment, but if you have a question, you can mention me in this PR (external contributors) or reach out in #docs-repo-reorg-support on Slack (internal contributors). |
buraizu
left a comment
There was a problem hiding this comment.
PR description doesn't disclose full scope of diff
The description states the edits are confined to updating-to-distribution-metrics-faq.md, but the diff also modifies rollup-for-distributions-with-percentiles.md and replaces the manage-tags.png image, none of which are mentioned.
Suggested fix: Update the PR description to reflect the actual files changed, or confirm the additional changes are intentional.
test
buraizu
left a comment
There was a problem hiding this comment.
PR description doesn't disclose full scope of diff
The description states the edits are confined to updating-to-distribution-metrics-faq.md, but the diff also modifies rollup-for-distributions-with-percentiles.md and replaces the manage-tags.png image, none of which are mentioned.
Suggested fix: Update the PR description to reflect the actual files changed, or confirm the additional changes are intentional.
test4
test5
| ## Rollup Interval Best Practices | ||
|
|
||
| It is recommended that a rollup interval be chosen based on the range being queried. As described above, the rollup `<aggregator>` parameter no longer affects distributions queried with percentiles, so only the `<time interval>` needs to be specified. |
There was a problem hiding this comment.
test1
Suggested fix: test2
|
|
||
| ## Does this update to rollup change the values of my existing queries on distributions with percentiles? | ||
| No - your existing queries are unaffected. The `.rollup <aggregator>` parameter has been removed because it had no effect on query results but caused user confusion for how the resulting value was calculated. | ||
| No - your existing queries are unaffected. We removed the `.rollup <aggregator>` parameter in order to reduce confusion, since it had no effect on query results but caused user confusion for how the resulting value was calculated. |
There was a problem hiding this comment.
"In order to" is wordier than "to," the sentence mentions confusion twice, and the switch to first-person "We removed" introduces a voice inconsistent with the rest of the page.
| No - your existing queries are unaffected. We removed the `.rollup <aggregator>` parameter in order to reduce confusion, since it had no effect on query results but caused user confusion for how the resulting value was calculated. | |
| test3 |
| - wildcards match every metric name that fits the pattern | ||
| - Review the estimated custom metric count before you save. | ||
| - percentile aggregations count toward your custom metrics total | ||
| - impact on existing tag configurations |
There was a problem hiding this comment.
The four list items mix grammatical forms (imperative, declarative, and a bare noun phrase) with inconsistent capitalization and end punctuation, making the list harder to scan.
Suggested fix: Wildcards match every metric name that fits the pattern.
|
|
||
| ## What has been updated for distributions with percentiles? | ||
| You no longer need to specify an additional time aggregator (as in `.rollup(avg)`) when you've already selected `p50`, `p75`, `p90`, `p95`, or `p99`. You only need to specify a rollup `<time interval>` which determines the interval of time your data is aggregated over. | ||
| You no longer need to utilize an additional time aggregator (as in `.rollup(avg)`) when you've already selected `p50`, `p75`, `p90`, `p95`, or `p99`. You only need to specify a rollup `<time interval>`, which easily determines the interval of time your data is aggregated over. |
There was a problem hiding this comment.
"Utilize" should be "use." "Easily determines" is an odd modifier placement — it implies the interval itself does something "easily," rather than the parameter making configuration easy.
| You no longer need to utilize an additional time aggregator (as in `.rollup(avg)`) when you've already selected `p50`, `p75`, `p90`, `p95`, or `p99`. You only need to specify a rollup `<time interval>`, which easily determines the interval of time your data is aggregated over. | |
| You no longer need to use an additional time aggregator (as in `.rollup(avg)`) when you've already selected `p50`, `p75`, `p90`, `p95`, or `p99`. You only need to specify a rollup `<time interval>`, which determines the interval of time your data is aggregated over. |
| ## Why has my distribution metrics page been deprecated? | ||
|
|
||
| Datadog has released a new pipeline for calculating globally accurate percentiles for your distribution metrics --- this unlocks more query functionality and an easier configuration workflow for distributions. | ||
| Datadog has released a new pipeline that you can utilize in order to calculate globally accurate percentiles for your distribution metrics. This gives you more query functionality and an easier configuration workflow for distributions. |
There was a problem hiding this comment.
"Utilize" and "in order to" are both unnecessarily wordy substitutes for "use" and "to."
| Datadog has released a new pipeline that you can utilize in order to calculate globally accurate percentiles for your distribution metrics. This gives you more query functionality and an easier configuration workflow for distributions. | |
| Datadog has released a new pipeline that you can use to calculate globally accurate percentiles for your distribution metrics. This gives you more query functionality and an easier configuration workflow for distributions. |
| - Boolean filtered queries are now available with distribution metrics with percentiles, so that you can write queries like: `p99:request_latency_distribution{app:A OR app:B} by {app}`. | ||
| - **Easier tag configuration and percentile aggregation workflows**: | ||
| - All tag configuration-percentile aggregations are managed in one centralized location on the [Metrics Summary][1] page. | ||
| - All tag configuration-percentile aggregations are managed in one centralized location on the [Metrics Summary][1] page, so you can easily leverage them across your organization. |
There was a problem hiding this comment.
"Leverage" is jargon for "use," and "easily" is an unnecessary filler adverb that doesn't add information.
| - All tag configuration-percentile aggregations are managed in one centralized location on the [Metrics Summary][1] page, so you can easily leverage them across your organization. | |
| - All tag configuration-percentile aggregations are managed in one centralized location on the [Metrics Summary][1] page, so you can use them across your organization. |
| {{< img src="metrics/faq/sketch-db-migration-faq/configure-tags.png" alt="Configure tags">}} | ||
|
|
||
| Click the {{< ui >}}Manage Tags{{< /ui >}} button to configure which tags are queryable on a particular metric. You can preview the new number of indexed custom metrics from your tag configuration before saving it. | ||
| Click the {{< ui >}}Manage Tags{{< /ui >}} button to configure which tags are queryable on a particular metric. Our UI allows you to preview the new number of indexed custom metrics from your tag configuration before it is saved by you. |
There was a problem hiding this comment.
"Our UI" uses first person where the rest of the page speaks directly to the reader about the product. "Before it is saved by you" is an awkward passive construction; the active form reads more naturally.
| Click the {{< ui >}}Manage Tags{{< /ui >}} button to configure which tags are queryable on a particular metric. Our UI allows you to preview the new number of indexed custom metrics from your tag configuration before it is saved by you. | |
| Click the {{< ui >}}Manage Tags{{< /ui >}} button to configure which tags are queryable on a particular metric. You can preview the new number of indexed custom metrics from your tag configuration before you save it. |
| @@ -8,10 +8,10 @@ is_beta: false | |||
| Time aggregation `(.rollup(<aggregator>, <time interval>)` and how it works with distributions with percentile aggregations has changed. | |||
|
|
|||
| ## Does this update to rollup change the values of my existing queries on distributions with percentiles? | |||
There was a problem hiding this comment.
| ## Does this update to rollup change the values of my existing queries on distributions with percentiles? | |
| test6 |
[DO NOT MERGE] Test fixture for docs on-call dashboard
This PR exists only to exercise the docs team dashboard's PR review composer. It intentionally introduces editorial issues into a single FAQ page. Do not merge it. Close it once dashboard testing is finished.
What's here
Five deliberate edits to
content/en/metrics/faq/updating-to-distribution-metrics-faq.md, chosen to produce a specific mix of review findings:Three that should produce inline comments with direct replacement suggestions:
utilizeandin order to(word-list swaps).easilyandleverage. This one is indented two spaces on purpose, to verify suggestion blocks preserve leading indentation.Our,allows you to, and a passive trailing clause.Two that should produce general (review-body) comments:
## Percentile Configuration Best Practicessection — heading breaks the file's question pattern and uses title case instead of sentence case; body is passive throughout and restates content from an earlier section.No new technical claims are introduced; edits 4 and 5 restate behavior already documented on the page.
Ready for merge