Add ability to choose what instrumentations are enabled #958
Conversation
grcevski
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #958 +/- ##
==========================================
+ Coverage 75.92% 80.02% +4.10%
==========================================
Files 132 133 +1
Lines 10378 10522 +144
==========================================
+ Hits 7879 8420 +541
+ Misses 1967 1584 -383
+ Partials 532 518 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -176,6 +179,8 @@ type metricsReporter struct { | |||
| bgCtx context.Context | |||
| ctxInfo *global.ContextInfo | |||
|
|
|||
| is instrumentations.InstrumentationSelection | |||
There was a problem hiding this comment.
Woulnd't make sense to use Features instead of new field? Features is enabling or disabling group of metrics anyway, so I wonder if this would led to confusion as we another param to disable more metrics.
Perhaps Instrumentations should be a nested property of Features. Maybe in the future we want also disable some metrics from network observability.
I think we need another level, which is consistent between metrics and traces. Features is right now only for metrics and what kind of metrics to expose, application, network, spans metrics... We need to be able to say, I only care about "http", don't pollute my metrics with sql or grpc, those I only want to see in traces. For example, SQL is managed by a cloud provider and you already export metrics from there. What kinds of instrumentations are captured is a separate dimension, I think we could make it part of features like this: application_http, application_sql..., but it gets very long and it's not user friendly IMO. I think we'd want to extend this concept to network and process metrics. Especially in process metrics, you may want to say I only care about "network", don't care about cpu or memory. |
grcevski
changed the title
| @@ -297,7 +297,7 @@ func (me *procMetricsExporter) Do(in <-chan []*process.Status) { | |||
| } | |||
|
|
|||
| func (me *procMetricsExporter) observeMetric(reporter *procMetrics, s *process.Status) { | |||
| me.log.Debug("reporting data for record", "record", s) | |||
| // me.log.Debug("reporting data for record", "record", s) | |||
There was a problem hiding this comment.
Did you forget to uncomment it?
There was a problem hiding this comment.
Ah yeah, these few that I commented out create a lot of log lines under debug, it was hard to see what's going on. I can put them back, but if process metrics are enabled it's really hard to follow the logs.
There was a problem hiding this comment.
Then I will remove it in another PR.
| @@ -939,6 +957,24 @@ the OpenTelemetry exporter will automatically add the `/v1/traces` path to the U | |||
| addition, you can use either the `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` environment variable or the `environment` YAML | |||
| property to use exactly the provided URL without any addition. | |||
|
|
|||
| | YAML | Environment variable | Type | Default | | |||
There was a problem hiding this comment.
Is this wrongly duplicated or it's a different section?
There was a problem hiding this comment.
They are different sections. We can specify which instrumentations should be enabled in each of the OTel metrics, Prometheus metrics, Traces. The variable names are different, following the similar naming conventions as the other variables.
This PR adds support for choosing what instrumentations should be enabled for traces, metrics, prometheus metrics. The users can define a subset from all available instrumentations for each export category. For example, all instrumentations can be enabled for traces, but only HTTP and SQL for metrics.
TODO: