-
Notifications
You must be signed in to change notification settings - Fork 52
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
[RFC] 3.0? #82
Comments
I would want to make sure that 2.0 supports Prometheus out of the gate, since it seems to be the defacto OSS solution these days. We could have a reporter that supported push gateway and the scrape endpoint. |
I want to consider deprecating this project in favor of: https://github.com/open-telemetry/opentelemetry-js, which covers metrics and tracing. CC: @mayurkale22, @dyladan, @OlivierAlbertini, @obecny It seems like it might be a better use of my time to focus my efforts on that project rather than re-invent the wheel a bit over here. |
@mayurkale22, @dyladan, @OlivierAlbertini, @obecny, does open-telemetry have gauges, timers, meters I am only really seeing counters here: https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-api/src/metrics/Meter.ts? Where as libs such as Node-Measured, Dropwizard, Micrometer, etc have more metric types https://github.com/yaorg/node-measured/tree/master/packages/measured-core#metric-implemenations |
@fieldju we would definitely be happy to have your help on opentelemetry js. While true that the metrics sdk is only seeing about 1000 downloads weekly right now, it is still in beta phase and is somewhat incomplete. I would say if you are going to deprecate this library in favor of otel, you should likely familiarize yourself with the state of the project and consider waiting until we launch GA toward the end of the summer before beginning to actively push your users to it. We may also be able to work on some sort of compatibility bridge similar to what we have for opentracing on the trace side that would allow your users to take advantage of opentelemetry in stages without making too many code changes all at once.
Metrics is under active development right now. You can see the full details of what will be supported in the spec here. Without knowing exactly how you define the difference between a gauge and a meter it is difficult to answer exactly, but you are probably looking for the ValueRecorder and the ValueObserver. There is currently not support or specification for timers, but I believe there has been active discussion in the specification around that. |
Maybe @jmacd can weigh in on the likelihood of adding timers to otel? He has been the major champion of the metrics spec. |
When I say timers I am mainly talking about the ability to do something like keeping track of the number of HTTP requests, how long they take. This is an example of what that looks like to Prometheus when it is scrapped: https://github.com/armory-plugins/armory-observability-plugin/blob/master/common/src/test/resources/io/armory/plugin/observability/prometheus/expected-scrape.txt#L1-L7 Here is the test that produces that scrape: https://github.com/armory-plugins/armory-observability-plugin/blob/master/common/src/test/java/io/armory/plugin/observability/prometheus/PrometheusScrapeControllerFunctionalTest.java#L59-L66 This lib does a similar thing: https://github.com/yaorg/node-measured/blob/master/packages/measured-node-metrics/lib/nodeHttpRequestMetrics.js#L17-L38 |
Ah yea you are looking for the ValueRecorder and ValueObserver which does MinMaxLastSumCount aggregation by default |
I have argued for some kind of timing instrument, and Bogdan has always opposed it. open-telemetry/opentelemetry-specification#464 I think without a builtin timing instrument, users are likely to get the units incorrect or misuse the clock in some way. A separate question is whether there should be some kind of "StopWatch" API that performs the measurement and records the timing, but I was only proposing an instrument for recording timings. In the Go API it would accept |
I would vote leave it as-is and encourage users to migrate. There are some major users that I'm sure would appreciate not needing to move so immediately and hopefully not needing to move twice. (once to v2 and then again to otel) |
@fieldju Thanks for taking great care of this library! I no longer rely on this day-to-day (different client, different stack now) and was very inactive after my initial contributions. I also think deprecating via npm (or leaving as is but making deprecation clear in the docs/README) is probably best. |
I am thinking I want to port this lib to Typescript and add some more integrations namely Prometheus.
Maybe finally clean up somethings on the Metrics objects, such as
toJson()
being the primary manner of getting data when half the time it doesn't return JSON.As I am basically the only one care and feeding this lib, I will leave this issue open for a while and see if anyone has comments and see if I have time to start this process.
Closes #32
In the process maybe we can get some integration tests up and running so that PRs like dependency auto bumps can be streamlined and we can make sure we stay up to date to prevent security issues.
I think porting to TS can be done 1 package at a time starting with core.
I am going to CC anyone who has ever contributed for visibility:
CC: @felixge, @mantoni, @Qard, @oliverzy, @csabapalfi, @dohse, @anacasner, @horte, @bnoordhuis, @tuckbick, @tlisonbee, @joeybaker, @hugebdu, @avolfson, @arlolra, @OlegIlyenko
The text was updated successfully, but these errors were encountered: