Skip to content

fix(internal): Corrections for DSC metrics#1491

Merged
jan-auer merged 1 commit intomasterfrom
fix/tsc-metrics
Sep 22, 2022
Merged

fix(internal): Corrections for DSC metrics#1491
jan-auer merged 1 commit intomasterfrom
fix/tsc-metrics

Conversation

@jan-auer
Copy link
Copy Markdown
Member

@jan-auer jan-auer commented Sep 22, 2022

Follow-up to #1466 that fixes some outlier conditions:

  • Assume a 0% change vs propagations ratio when there were no changes
  • Log timing metrics even if there was no change (using the start timestamp)
  • Skip changes at the end of every transaction (i.e. event processors)

cc @ale-cota @AbhiPrasad

#skip-changelog

@jan-auer jan-auer requested a review from a team September 22, 2022 10:52
@jan-auer jan-auer self-assigned this Sep 22, 2022
let percentage = ((change as f64) / (total as f64)).min(1.0);
let percentage = match (change, total) {
(0, 0) => 0.0, // 0% indicates no premature changes.
_ => ((change as f64) / (total as f64)).min(1.0) * 100.0,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Doing the x100 here instead of on the presentation layer seems wrong.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agreed, though I have not found a way to do this in Datadog. If you prefer, we can also keep it between 0-1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine - the name says "percentage", so multiplying by 100 does make the number consistent with the metric name.

@jan-auer jan-auer merged commit d97ff20 into master Sep 22, 2022
@jan-auer jan-auer deleted the fix/tsc-metrics branch September 22, 2022 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants