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

metrics, add support for auto collecting default recommended metrics #623

Closed
shaz1v opened this issue Dec 15, 2019 · 13 comments
Closed

metrics, add support for auto collecting default recommended metrics #623

shaz1v opened this issue Dec 15, 2019 · 13 comments

Comments

@shaz1v
Copy link

shaz1v commented Dec 15, 2019

Is your feature request related to a problem? Please describe.

There are a lot of default metrics which are recommended and should be collected and exported in every node project running, implementing the manual collection of each one of them in every project could be solved by adding support in the lib, and by that engaging metrics lib as a lot more noob friendly.

Describe the solution you'd like

Add support for auto collecting default recommended metrics, sort of like all the plugins for the tracing lib, add a default metrics plugin, for the metrics lib.

Describe alternatives you've considered

Collecting all those metrics manually, (which is what I'm doing currently), although it would be nice (and super noob friendly) to add support for auto collecting those metrics.

Additional context

Prometheus has a list of recommended metrics to export,
https://prometheus.io/docs/instrumenting/writing_clientlibs/#standard-and-runtime-collectors

Plus some node-specific ones from the prom-client library
https://github.com/siimon/prom-client/tree/29b6d86c3c8246c8e7f14fcc39a16682e2248934/lib/metrics

Thanks!

@mayurkale22 mayurkale22 added this to the Alpha v0.4 milestone Dec 17, 2019
@mayurkale22
Copy link
Member

+1

But I think we should wait until we have implemented full metrics SDK with aggregation support (#402 and #452). Also, I was thinking to create a dedicated package for default recommended metrics (one option is @opentelemetry/system-metrics)

@bhupesh-sf
Copy link

+1

Something like this will be great Swagger-stats

@dyladan
Copy link
Member

dyladan commented Oct 26, 2020

The metrics sdk is still heavily underspecified and our implementation is an incomplete version of that incomplete specification. In my opinion it would be a mistake to implement too much on top of the current implementation.

@bhupesh-sf
Copy link

Any update here for good metrics support?

@dyladan

@vmarchaud
Copy link
Member

@bhupesh-sf There is the host-metrics package available and an open PR here for nodejs metrics.

As the specification is still moving (and i believe the current focus of the specification group), the comment of @dyladan still stand, we should avoid implementing too much on top of a moving API.

@bhupesh-sf
Copy link

@vmarchaud Its great to see these packages. We are really moving towards what is really desired.

As I mentioned earlier a package https://github.com/slanatech/swagger-stats gives lots of great metrics enough for our almost all use cases. Right now we are using this for our project but would really like to have something like that available within opentelemetry.

Keep us posted 👍

@marcbachmann
Copy link
Contributor

I just did a small POC in https://github.com/marcbachmann/opentelemetry-node-metrics by extracting some code from https://github.com/siimon/prom-client

@marcbachmann
Copy link
Contributor

The module mentioned above got migrated to bound instruments and is working properly.
I've published it to npm as opentelemetry-node-metrics. I hope this can ease the adoption of @opentelemetry/metrics as long as the other PRs aren't merged.

@legendecas
Copy link
Member

Closing as https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/packages/opentelemetry-host-metrics is published.

@Luckz
Copy link

Luckz commented Aug 21, 2022

Would argue merely having "host metrics" does not "complete" anything like "default recommended (application) metrics".
Obviously goes for #955 too, which for example specifically mentions "http requests" and "like Spring Boot Actuator". #1330 also asks for "http request count".

@legendecas
Copy link
Member

@Luckz HTTP instrumentation metrics are being worked in PR #3149.

@Luckz
Copy link

Luckz commented Aug 22, 2022

Yes, I tried to run commit fdd6f58 too but didn't manage to make it work.

However, that is HTTP as in Node's http package for HTTP/HTTPS, and thus also not necessarily what those issues were asking for.

@legendecas
Copy link
Member

However, that is HTTP as in Node's http package for HTTP/HTTPS, and thus also not necessarily what those issues were asking for.

@Luckz I'm not sure I fully understand your point clearly. We are working towards adding specified metric signals in the instrumentation packages. Requesting new metric signals is always welcome.

I don't think we are going to include these signals in the @opentelemetry/sdk-metrics, same as not generating trace signals in @opentelemetry/sdk-traces-base. However, we are bundling several commonly used instrumentation in @opentelemetry/sdk-node, so that common signals are generated when using @opentelemetry/sdk-node when little configuration.

pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants