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

[DOCS-6807] Historical Metrics Ingestion #20809

Merged
merged 71 commits into from
Feb 5, 2024
Merged

Conversation

mitheysh-asokan
Copy link
Contributor

@mitheysh-asokan mitheysh-asokan commented Nov 28, 2023

What does this PR do? What is the motivation?

Creates Documentation for Historical Metrics Ingestion Preview

Merge instructions

  • Please merge after reviewing

Additional notes

@mitheysh-asokan mitheysh-asokan requested a review from a team as a code owner December 19, 2023 18:23
Copy link
Contributor

@ijkaylin ijkaylin left a comment

Choose a reason for hiding this comment

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

initial round of review; please make the updates and let me know when it's ready for round 2 review

Copy link
Contributor

Choose a reason for hiding this comment

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

To me, instead of having the previous state vs current state (with HMI) enabled being linearly presented top down, i feel like the message would be clearer if you presented it side by side (no HMI enabled vs HMI enabled - left right respectively)

The message of "metric point delayed by 1h & 1 min or 3hrs" is in red and distracts the user with the question of why is my point delayed? is it dd's fault? My suggested edit is to change "metric point delayed by 1hr&1min" to "historical data's timestamp from 1h&1min ago" or something along those lines

Another comment I have is that the DD icon appears after the purple box of metrics intake...which from a customer's perspective is DD. Could we have a larger boundary to encapsulate what's actually on the DD side vs being submitted by the customer ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mitheysh-asokan for some reason this comment doesn't show up on the files we reviewed. Wanted to make sure you saw this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did speak to our designer about this, but it sounds like a side-by side for public docs might not be an optimal viewing experience - due to some length limitations. He advises on making the section seperations to be more prominent instead of making them side-by-side.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested Title: "Ingesting historical metrics data can take longer depending on the timestamp"

Comments:

  • Again metrics intake is a part of DD -- right now the image looks like we do edge processing before it reaches the rest of DD which isn't the point of the image.
  • For metric A's points that have timestamps from 3 months ago or 15 months ago, it's unnecessary IMO to include the hourly timestamp -- just adds visual clutter
  • Same comment as above by the term "delayed"

Copy link
Contributor

Choose a reason for hiding this comment

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

content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
estherk15 and others added 2 commits January 3, 2024 15:04
Co-authored-by: Kathy L. <kathy.lin@datadoghq.com>
Copy link
Contributor

@ijkaylin ijkaylin left a comment

Choose a reason for hiding this comment

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

I think it'd be worthwhile for the 3 of us to hop on a quick 15 minute sync and whiteboard what changes need to be made to the diagrams before we can publish these public docs. The current pictures are not illustrating the functionality in an effective way.

content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
| +30 days | +14 hours latency |


{{< img src="metrics/custom_metrics/historical_metrics/diagram_historical-metrics-ingestion_3_240105.png" alt="Diagram showing how Historical Metrics can take longer to ingest depending on the metric timestamp">}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Highlighting the historical data value's timestamp in pinkish red makes users nervous that there is something incorrect about their submission. Let's change this to black

Copy link
Contributor

Choose a reason for hiding this comment

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

Resolution from mtg - remove the diagram, the table provides sufficient information

content/en/metrics/custom_metrics/historical_metrics.md Outdated Show resolved Hide resolved
@estherk15 estherk15 merged commit ef83cf0 into master Feb 5, 2024
11 checks passed
@estherk15 estherk15 deleted the mitheysh.asokan/LateMetrics branch February 5, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Everything related to the Doc backend editorial review Waiting on a more in-depth review Images Images are added/removed with this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants