-
Notifications
You must be signed in to change notification settings - Fork 783
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
Replace OpenTelemetry with armon/go-metrics #1395
Comments
Would it be better to revert #1378 and start at this fresh or convert the opentel code that is already there? |
@eikenb good question, I think reverting would be most clear for tracing this pivot when looking back at the history. The original PR could still be used as a reference when making the new changes. |
Ok, sounds good. I'll get it reverted. Be aware that, because we'll be reverting a merge, if you continue the work off the reverted code you'll need to revert the revert before merging the new code. See this doc for the details... https://github.com/git/git/blob/master/Documentation/howto/revert-a-faulty-merge.txt |
Opentelemetry merge reverted: 6bc149b |
Any updates? |
Hi @Oloremo, no specifics yet. We got sidetracked launching a new tool which ate all our time for a while. If you'd like this to get priority, please 👍 the original issue. We use those to help gauge community priority and how they figure into our prioritized backlog. Thanks. |
Yeah currently consul-template in a weird state, hashicorp propose it as a critical part of the Nomad infra for example: https://learn.hashicorp.com/tutorials/nomad/vault-pki-nomad yet it's not very clear how to monitor it. |
Opps. Didn't click through on that vault-pki-nomad link and mistook it for a different case. So ignore the text below (leaving for context). But please be assured we are working on these things. And thanks for adding the 👍. -- Yeah. It is kind of, but we are working on fixing it with the refactoring of the library case out of consul-template. Once done (I'm actively working on this) it should be a big help as it cleans things up immensely for those use cases (the 'using consul-template as a library' cases). You can follow along at https://github.com/hashicorp/hcat if your interested. |
Interesting! Does that mean that the consul-template will be sunsetted soon? |
As a library, yes. As a stand alone application, no. We'll be updating it to use the new library, but it won't be going anywhere as an application at this point. We're considering merging it and envconsul which, once merged, might end up under a new name, but we're not 100% on that yet. |
Thanks for the clarification! |
So it missed the 0.26.0... Will be in 0.27?.. |
Hey @Oloremo, Sorry that this didn't make it into 0.26.0, I'm working on getting caught up and only wanted to focus on existing PRs and fixing bugs at this point. After this release I need to make a pass of the other projects I maintain (envconsul, consul-esm) and then I'll be putting together some roadmaps for the projects and will determine the priority of this at that time. If you (and readers) want this, please consider adding a 👍 the top/original issue as I'll look at those when working on the priorities for the roadmap. The roadmaps will also be public and feedback will be welcome there as well. Thanks. |
Hello @eikenb ! I am currently looking into implementing metrics in Consul-Template using armon/go-metrics, as requested in the issue, however I am a bit stuck regarding the implementation of histogram metrics: Do you have any opinion regarding the next steps for this issue? Whether it being to ping the maintainer of the aforementioned PR (which I'll do) or to switch to another metric library ? |
Imo, if the feature is missing it makes sense to implement everything but those metrics and update the metrics after the needed feature will be provided. Something is better than nothing. |
Summary
After a few months of weighing the current state of OpenTel vs the current need for metrics wrt to the Consul ecosystem, it has become more evident to me that switching to
armon/go-metrics
would provide the most cohesive and comprehensive feature set at this time.Background
PR #1378 recently introduced metrics to Consul Template using OpenTelemetry. There's a short blurb providing context as to why OpenTel was initially chosen over
armon/go-metrics
and that it was an experiment towards a new library for telemetry.Context
I tried to create a uniform UX for configuring and emitting metrics over on Consul ESM #70, another Consul ecosystem project, and quickly ran into some short comings when dealing with more complex metric types than counters and histograms. For example, timer values were reduced to histogram integers, and there currently is not a systematic way to denote measurement (seconds, milliseconds, etc).
The community request for
statsd
is not quite satisfied yet due to the difference in tag syntax between statsd variants and would require omitting the use of tags as a whole to continue support for statsd.UX
The metrics reported should not change much from the initial work in #1378 (not yet released). However, the telemetry configuration will be breaking changes compared to the PR and align closer to how it is supported by Consul (docs).
Switching libraries will add feature support for:
The text was updated successfully, but these errors were encountered: