Skip to content

Conversation

@ddtmachado
Copy link
Contributor

@ddtmachado ddtmachado commented Dec 17, 2019

What does this PR do?

Fixes #5944

Motivation

StatsD assumes that duration based metric values are in Milliseconds.

Currently Traefik always calls the Observe function converting the time.Duration to seconds. With this change we can set the scale in the metric provider implementation, on statsD case Milliseconds, while keeping the current value as the default for those that don't specify it.

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

@mpl

This comment has been minimized.

Copy link
Collaborator

@mpl mpl left a comment

Choose a reason for hiding this comment

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

It looks like you forgot to fix something related in the tests: SimpleSuite.TestMetricsPrometheusDefaultEntryPoint is panicking, according to Travis CI.

Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

Thanks 👍

Copy link
Collaborator

@jbdoumenjou jbdoumenjou left a comment

Choose a reason for hiding this comment

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

LGTM

@traefiker traefiker merged commit b5d205b into traefik:v2.1 Mar 5, 2020
@traefiker traefiker mentioned this pull request Mar 5, 2020
@ddtmachado ddtmachado deleted the GH-5944-statsd branch September 8, 2022 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants