[prometheusremotewrite] add otel_scope_info metric to prometheus remote write exporter #24248

blakeroberts-wk · 2023-07-12T22:33:58Z

Description:

These changes add scope name and version to a metric data point as labels. Additionally, an otel_scope_info metric may be emitted if the scope contains additional attributes. Similar to the target_info metric, the otel_scope_info metric can be joined with a metric on job, instance, otel_scope_name, and otel_scope_version to retrieve instrumentation scope attributes.

These changes align the prometheus remote write exporter with the following specification update: open-telemetry/opentelemetry-specification#2703

Link to tracking Issue: #21091

Testing:

I pulled these changes into a collector which accepts OTLP metrics from many java agents and exports data to a prometheus compatible backend.

// series 1
http_server_active_requests{
http_method="GET",
http_scheme="http",
instance="REDACTED_1",
job="REDACTED_2",
net_host_name="REDACTED_3",
net_host_port="8080",
otel_scope_name="io.opentelemetry.servlet-5.0",
otel_scope_version="1.26.0-alpha"
}
// series 2
http_server_active_requests{
http_method="GET",
http_scheme="http",
instance="REDACTED_1",
job="REDACTED_2",
net_host_name="REDACTED_3",
net_host_port="8080",
otel_scope_name="io.opentelemetry.tomcat-10.0",
otel_scope_version="1.26.0-alpha"
}

After deployment, errors from the backend around samples with duplicate timestamps ceased.

Documentation:

The prometheus remote write exporter's README was updated with the new configuration setting to enable/disable the creation of the otel_scope_info metric. Like the target_info metric, I've enabled this feature by default which may or may not be appropriate.

exporter/prometheusremotewriteexporter/factory.go

pkg/translator/prometheusremotewrite/helper.go

pkg/translator/prometheusremotewrite/helper_test.go

…ote write exporter

blakeroberts-wk · 2023-07-25T21:34:50Z

@evan-bradley would you review my changes, please? Thank you.

evan-bradley · 2023-07-31T20:25:26Z

@blakeroberts-wk Sorry for the delay. Overall what you have looks good, but I don't know that I can answer the question of how we should handle emitting an Instrumentation Scope metric or name/version attributes if we don't know the name of the scope.

Please add a changelog entry for this change and fix the branch conflict.

blakeroberts-wk · 2023-08-02T15:46:29Z

@evan-bradley I have updated the branch and added a change log entry. are there any more concerns around these changes?

pkg/translator/prometheusremotewrite/helper.go

.chloggen/O11Y-3152.yaml

pkg/translator/prometheusremotewrite/helper.go

evan-bradley · 2023-08-02T16:57:57Z

@evan-bradley I have updated the branch and added a change log entry. are there any more concerns around these changes?

I don't have any concerns, but before we can merge a functional change like this we need approval from one of the code owners.

@Aneurysm9 @rapphil Please take a look when you are able.

rapphil · 2023-08-03T06:29:40Z

pkg/translator/prometheusremotewrite/helper.go

+		return
+	}
+	if scope.Attributes().Len() == 0 {
+		// If the scope doesn't have additional attributes, then otel_scope_info isn't useful.


It will contain the name and version of the instrumentation scope. Isn't this information per se useful?

It will contain the name and version of the instrumentation scope. Isn't this information per se useful? It means that there is at least a single metric being emitted in that scope.

Having scope_info in the configuration enabled and not generating the otel_scope_info metric is misleading and will confuse users.

looking into the compliance matrix, it seems that only PHP and rust will be able to set the scope using the Otel API?

get_meter accepts attributes.

So one more point to for users to have the least surprise by always emitting this metric?

Isn't this information per se useful? It means that there is at least a single metric being emitted in that scope.

If one was looking for metrics under a specific scope, they'd probably do something like a topk query on

__name__{otel_scope_name="w/e"}

In general, actual usage of otel_scope_info is most likely after knowing the __name__ you're after then

<query> / on (otel_scope_name, otel_scope_version) group_left otel_scope_info

Though the query would return no data if otel_scope_info doesn't exist. So if the feature is enabled, it might make sense to always report it so queries that use the metric always return data regardless of whether or not the joined labels add novel information

I pushed a commit that emitted the metric regardless.

But now I'm second guessing it. This has the potential to add a lot of active time series that won't be valuable the majority of the time. The collector could receive metrics from many languages and only PHP and rust even have the potential to make this metric add novel information. And the query with the join tends to be a rather advanced use case (so advanced that documentation doesn't exist around how to properly use otel_scope_info let alone target_info) Additionally, not emitting the metric if it doesn't have attributes would align with the implementation of target_info: https://github.com/blakeroberts-wk/opentelemetry-collector-contrib/blob/3f146d90bd9c622c33359c71fcae89162a263f2c/pkg/translator/prometheusremotewrite/helper.go#L589-L591

rapphil · 2023-08-03T06:31:18Z

pkg/translator/prometheusremotewrite/metrics_to_prw.go

 					}
 				default:
 					errs = multierr.Append(errs, errors.New("unsupported metric type"))
 				}
 			}
+			addScopeTargetInfo(scope, resource, settings, mostRecentTimestamp, tsMap)


nit, should make the signature follow the hierarchy: addScopeTargetInfo(resource, scope, settings, mostRecentTimestamp, tsMap)?

The collector doesn't implement this yet and the metrics are not very useful. See: open-telemetry/opentelemetry-collector-contrib#24248 Signed-off-by: Goutham <gouthamve@gmail.com>

* Disable scope_info by default The collector doesn't implement this yet and the metrics are not very useful. See: open-telemetry/opentelemetry-collector-contrib#24248 Signed-off-by: Goutham <gouthamve@gmail.com> * Add changelog entry Signed-off-by: Goutham <gouthamve@gmail.com> --------- Signed-off-by: Goutham <gouthamve@gmail.com>

github-actions · 2023-08-29T05:20:29Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

jpkrohling · 2023-09-05T14:34:35Z

@rapphil, is there anything blocking this PR from being merged?

evan-bradley · 2023-11-16T20:58:03Z

@blakeroberts-wk Looks like some of the tests are failing, can you try rebasing/merging in main?

@Aneurysm9 @rapphil could you take a look as the code owners?

blakeroberts-wk · 2023-11-16T21:26:35Z

I have merged main.

The unit test TestDeleteAfterRead_SkipPartials within github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer passes when ran locally.

Can the CI check be re-ran or skipped? @evan-bradley

blakeroberts-wk · 2023-12-11T20:28:55Z

@evan-bradley are we still feeling confident in these changes?

evan-bradley · 2023-12-11T21:32:41Z

I don't see any issues with these changes, but we need an approval from the code owners of this component.

@Aneurysm9 @rapphil could you please take a look?

github-actions · 2023-12-26T05:19:11Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

evan-bradley · 2024-01-02T19:14:59Z

@Aneurysm9 @rapphil Please take a look.

Aneurysm9 · 2024-01-12T20:17:17Z

I think this is missing allowance for the use case where metrics are ingested with the prometheus receiver and expected to be exported without modification. This is similar to the discussions previously regarding the normalization of metric names that do not necessarily make sense when receiving and exporting prometheus data. Configuration allowing the operator to control this behavior should be added.

blakeroberts-wk · 2024-01-12T22:38:16Z

I think this is missing allowance for the use case where metrics are ingested with the prometheus receiver and expected to be exported without modification.

This implementation will not add the labels if the scope name/version are empty. However, if the scope name/version are empty, the prometheus receiver adds a default name and version:

opentelemetry-collector-contrib/receiver/prometheusreceiver/internal/transaction.go

Lines 233 to 234 in 9e47c92

    
           ils.Scope().SetName(receiverName) 
        
           ils.Scope().SetVersion(t.buildInfo.Version)

.

I think it makes sense for the receiver to modify this data. But it does make it awkward to meet the expectation that prometheus data received by the collector isn't modified when exported in prometheus format. To be honest, I find this expectation to be silly. If I wanted to proxy prometheus data without modification, I'd use a prometheus server. If I wanted to proxy prometheus data with modification (which I do in practice), I'd use the otel collector. So to use the otel collector with the intent to modify prometheus data, I'd argue it'd be unreasonable for me to expect that data won't be modified. To be clear, modifications to the metric name seem intrusive. But adding labels used to appropriately identify a unique time series seems like reasonable, if not expected, modification.

Furthermore, the design docs of the prometheus receiver make the aforementioned expectation impossible:

It doesn't need to provide one-to-one mapping

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/prometheusreceiver/DESIGN.md#map-prometheus-metrics-to-the-corresponding-opentelemetry-metrics-properly

Because the prometheus receiver does not guarantee data is mapped one-to-one, the prometheus remote write exporter cannot guarantee its output matches whatever input was given to the receiver.

Configuration allowing the operator to control this behavior should be added.

This seems reasonable, but low value. Would it be acceptable to open an issue about adding this configuration?

github-actions · 2024-01-27T05:18:14Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

github-actions · 2024-02-15T05:19:06Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

github-actions · 2024-03-01T05:19:08Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

github-actions · 2024-03-15T05:19:09Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

gespi1 · 2024-07-19T14:53:35Z

@evan-bradley @blakeroberts-wk what is the status of these changes, are they being superseded by another PR elsewhere or traction has stopped and still waiting on @Aneurysm9 ?

evan-bradley · 2024-07-19T15:30:07Z

I'm sorry, I lost track of this. This change was still dependent on the code owners' approval. I'm not aware of any other PRs that have superseded it.

add otel_scope_info metric to prometheus remote write exporter

07e26dd

blakeroberts-wk requested a review from a team July 12, 2023 22:33

blakeroberts-wk requested review from Aneurysm9 and kovrus as code owners July 12, 2023 22:33

github-actions bot added exporter/prometheusremotewrite pkg/translator/prometheus labels Jul 12, 2023

github-actions bot requested a review from rapphil July 12, 2023 22:34

github-actions bot assigned evan-bradley Jul 12, 2023

Aneurysm9 reviewed Jul 17, 2023

View reviewed changes

exporter/prometheusremotewriteexporter/factory.go Outdated Show resolved Hide resolved

pkg/translator/prometheusremotewrite/helper.go Outdated Show resolved Hide resolved

pkg/translator/prometheusremotewrite/helper_test.go Show resolved Hide resolved

blakeroberts-wk added 3 commits July 17, 2023 12:37

default otel_scope_info metric generation to false for prometheus rem…

c0c0acd

…ote write exporter

Merge branch 'main' into O11Y-3152

000bb0b

fix lints and tests

b7a328b

blakeroberts-wk added 3 commits August 1, 2023 11:14

Merge branch 'main' into O11Y-3152

57840c1

add changelog entry

dabcd5d

Merge branch 'main' into O11Y-3152

8814c5a

dashpole approved these changes Aug 2, 2023

View reviewed changes

pkg/translator/prometheusremotewrite/helper.go Outdated Show resolved Hide resolved

pkg/translator/prometheusremotewrite/helper.go Outdated Show resolved Hide resolved

evan-bradley approved these changes Aug 2, 2023

View reviewed changes

.chloggen/O11Y-3152.yaml Outdated Show resolved Hide resolved

pkg/translator/prometheusremotewrite/helper.go Outdated Show resolved Hide resolved

rapphil reviewed Aug 3, 2023

View reviewed changes

gouthamve mentioned this pull request Aug 10, 2023

otel/prometheus_exporter: disable otel_scope_info by default grafana/agent#4775

Merged

4 tasks

emit otel_scope_info metric when scope doesn't contain attributes

3f146d9

github-actions bot added the Stale label Aug 29, 2023

blakeroberts-wk added 2 commits November 2, 2023 09:06

Merge branch 'main' into O11Y-3152

32f2736

Merge branch 'main' into O11Y-3152

a7a231c

Merge branch 'main' into O11Y-3152

8e36f96

blakeroberts-wk added 2 commits November 28, 2023 13:17

Merge branch 'main' into O11Y-3152

4c1546a

Merge branch 'main' into O11Y-3152

9250192

jmichalek132 mentioned this pull request Dec 15, 2023

Prometheus Exporter should add InstrumentationScope labels #21091

Closed

github-actions bot added the Stale label Dec 26, 2023

Merge branch 'main' into O11Y-3152

8a0eb23

github-actions bot removed the Stale label Dec 28, 2023

github-actions bot added the Stale label Jan 27, 2024

evan-bradley removed the Stale label Jan 31, 2024

github-actions bot added the Stale label Feb 15, 2024

dashpole removed the Stale label Feb 15, 2024

github-actions bot added the Stale label Mar 1, 2024

jmichalek132 mentioned this pull request Mar 10, 2024

Proposal to add otel.collector.receiver.name attribute open-telemetry/semantic-conventions#759

Open

github-actions bot closed this Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[prometheusremotewrite] add otel_scope_info metric to prometheus remote write exporter #24248

[prometheusremotewrite] add otel_scope_info metric to prometheus remote write exporter #24248

blakeroberts-wk commented Jul 12, 2023

blakeroberts-wk commented Jul 25, 2023

evan-bradley commented Jul 31, 2023

blakeroberts-wk commented Aug 2, 2023

evan-bradley commented Aug 2, 2023

rapphil Aug 3, 2023

rapphil Aug 3, 2023

blakeroberts-wk Aug 14, 2023

blakeroberts-wk Aug 14, 2023

rapphil Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 29, 2023

jpkrohling commented Sep 5, 2023

evan-bradley commented Nov 16, 2023

blakeroberts-wk commented Nov 16, 2023

blakeroberts-wk commented Dec 11, 2023

evan-bradley commented Dec 11, 2023

github-actions bot commented Dec 26, 2023

evan-bradley commented Jan 2, 2024

Aneurysm9 commented Jan 12, 2024

blakeroberts-wk commented Jan 12, 2024

github-actions bot commented Jan 27, 2024

github-actions bot commented Feb 15, 2024

github-actions bot commented Mar 1, 2024

github-actions bot commented Mar 15, 2024

gespi1 commented Jul 19, 2024

evan-bradley commented Jul 19, 2024

[prometheusremotewrite] add otel_scope_info metric to prometheus remote write exporter #24248

[prometheusremotewrite] add otel_scope_info metric to prometheus remote write exporter #24248

Conversation

blakeroberts-wk commented Jul 12, 2023

blakeroberts-wk commented Jul 25, 2023

evan-bradley commented Jul 31, 2023

blakeroberts-wk commented Aug 2, 2023

evan-bradley commented Aug 2, 2023

rapphil Aug 3, 2023

Choose a reason for hiding this comment

rapphil Aug 3, 2023

Choose a reason for hiding this comment

blakeroberts-wk Aug 14, 2023

Choose a reason for hiding this comment

blakeroberts-wk Aug 14, 2023

Choose a reason for hiding this comment

rapphil Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Aug 29, 2023

jpkrohling commented Sep 5, 2023

evan-bradley commented Nov 16, 2023

blakeroberts-wk commented Nov 16, 2023

blakeroberts-wk commented Dec 11, 2023

evan-bradley commented Dec 11, 2023

github-actions bot commented Dec 26, 2023

evan-bradley commented Jan 2, 2024

Aneurysm9 commented Jan 12, 2024

blakeroberts-wk commented Jan 12, 2024

github-actions bot commented Jan 27, 2024

github-actions bot commented Feb 15, 2024

github-actions bot commented Mar 1, 2024

github-actions bot commented Mar 15, 2024

gespi1 commented Jul 19, 2024

evan-bradley commented Jul 19, 2024

rapphil Aug 3, 2023 •

edited

Loading