[RFC] 3.0?
fieldju opened this issue · 12 comments
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
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.
Even though technically this project has been around longer as far as I can tell.
☝️ even though this lib has a lot more usage, it might still be worth depreciating and pointing users over to opentelemetry-js and having me just contribute to that lib. Since there is more dev activity over there and it can be better supported.
Also it is already written in Typescript :)
@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.
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?
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
Which creates a request histogram, https://yaorg.github.io/node-measured/packages/measured-core/global.html#HistogramData
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 time.Duration to ensure that users don't use the wrong units or clock API.
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.