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

Added configurable prefix for statsd metrics collection #5336

Merged
merged 3 commits into from Nov 12, 2019

Conversation

schulterklopfer
Copy link
Contributor

We installed multiple instances of traefik for request seperation. To monitor our setup in a better way, we needed configurable statsd prefixes, so the metrics won't end up in one big bucket :)

@MichaelErmer
Copy link
Contributor

Any news on this? this is a Roadblock for us to upgrade to 2.0, as we currently run a custom 1.7 with this feature.

@ldez ldez added this to To review in v2 via automation Oct 23, 2019
@ldez ldez changed the title Added configurable prefix for stats metrics collection Added configurable prefix for statsd metrics collection Oct 25, 2019
Copy link
Member

@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,

Could you add the documentation:

  • run go generate to generate CLI and env vars doc
  • add the option in docs/content/observability/metrics/statsd.md
  • add the option in docs/content/reference/static-configuration/file.toml
  • add the option in docs/content/reference/static-configuration/file.yaml

pkg/metrics/statsd.go Outdated Show resolved Hide resolved
@schulterklopfer
Copy link
Contributor Author

Do you need anything additional from me? I think I addressed the change requests in my latest commits. :)

Copy link
Member

@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
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@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

@mmatur
Copy link
Member

mmatur commented Nov 12, 2019

Hi @schulterklopfer,

Thanks for you contribution, unfortunately the bot is not able to merge the PR because, it is not allowed to rebase your PR on the master branch

Could you please rebase your PR on master, or give the right to maintainers to update your PR

@traefiker traefiker merged commit ca1d980 into traefik:master Nov 12, 2019
v2 automation moved this from To review to Done Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v2
Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants