[pkg/translator/prometheus] Add option to keep UTF-8 characters #35904

ArthurSens · 2024-10-21T19:41:43Z

Description

Updates the translator package to allow keeping UTF-8 characters. The intended usage of this is that components that use this package would add new configuration options to allow users to keep UTF-8 characters in metric and label names.

For the components that are using this package, I'm adding false as the argument to keep the current behavior. Follow-up PRs will be opened to allow configurability.

Link to tracking issue

Related to #35459

ArthurSens · 2024-10-30T13:44:03Z

@dashpole, I'm not sure what to do with the googlemanagedprometheusexporter 🤔, at least I don't understand how I can solve the problem in this PR.

Google's managed prometheus is doing some weird go mod replaces somewhere?

dashpole · 2024-10-30T14:53:09Z

Yikes. Previously, we've had to deprecate and replace functions that are changed. I might be able to refactor the GMP code to move usage of the translator package back into contrib.

ArthurSens · 2024-10-30T16:42:27Z

Yikes. Previously, we've had to deprecate and replace functions that are changed. I might be able to refactor the GMP code to move usage of the translator package back into contrib.

I could change the implementation into a new function BuildUTF8Name if you prefer, but it's weird that user of the library blocks updates like this 🤔

dashpole · 2024-10-31T19:33:39Z

working to unblock you: GoogleCloudPlatform/opentelemetry-operations-go#908

@ArthurSens

…-go to the latest release (#36132) #### Description Update github.com/GoogleCloudPlatform/opentelemetry-operations-go to https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/releases/tag/v0.49.0. This unblocks #35904 by moving the prometheus sanitization into contrib. @ArthurSens

ArthurSens · 2024-11-02T23:02:01Z

Thanks for unblocking this @dashpole ❤️, it should be ready for review

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

bertysentry · 2024-11-04T14:16:05Z

pkg/translator/prometheus/normalize_name_test.go

-	require.Equal(t, "unsupported_metric_redundant_test_per_C", normalizeName(createGauge("unsupported.metric.redundant", "__test $/°C"), ""))
-
+func TestAllowUTF8(t *testing.T) {
+	for _, allowUTF8 := range []bool{true, false} {


Please remove the for followed by if/else. Unnecessary complexity.

// Don't allow UTF-8 (default) require.Equal(t, "unsupported_metric_temperature_F", normalizeName(createGauge("unsupported.metric.temperature", "°F"), "", false)) require.Equal(t, "unsupported_metric_weird", normalizeName(createGauge("unsupported.metric.weird", "+=.:,!* & #"), "", false)) require.Equal(t, "unsupported_metric_redundant_test_per_C", normalizeName(createGauge("unsupported.metric.redundant", "__test $/°C"), "", false)) // Allow UTF-8 require.Equal(t, "unsupported.metric.temperature_°F", normalizeName(createGauge("unsupported.metric.temperature", "°F"), "", true)) require.Equal(t, "unsupported.metric.weird_+=.:,!* & #", normalizeName(createGauge("unsupported.metric.weird", "+=.:,!* & #"), "", true)) require.Equal(t, "unsupported.metric.redundant___test $_per_°C", normalizeName(createGauge("unsupported.metric.redundant", "__test $/°C"), "", true))

bertysentry · 2024-11-04T14:32:16Z

pkg/translator/prometheus/normalize_name_test.go

+	require.Equal(t, "system_cpu_utilization_ratio", normalizeName(createGauge("system.cpu.utilization", "1"), "", false))
+	require.Equal(t, "system_disk_operation_time_seconds_total", normalizeName(createCounter("system.disk.operation_time", "s"), "", false))
+	require.Equal(t, "system_cpu_load_average_15m_ratio", normalizeName(createGauge("system.cpu.load_average.15m", "1"), "", false))
+	require.Equal(t, "memcached_operation_hit_ratio_percent", normalizeName(createGauge("memcached.operation_hit_ratio", "%"), "", false))


Edge case: what's the expected result if allowedUTF8 == true?

It's still translated to percent

bertysentry · 2024-11-04T14:32:33Z

pkg/translator/prometheus/normalize_name_test.go

+	require.Equal(t, "mongodbatlas_process_journaling_data_files_mebibytes", normalizeName(createGauge("mongodbatlas.process.journaling.data_files", "MiBy"), "", false))
+	require.Equal(t, "mongodbatlas_process_network_io_bytes_per_second", normalizeName(createGauge("mongodbatlas.process.network.io", "By/s"), "", false))
+	require.Equal(t, "mongodbatlas_process_oplog_rate_gibibytes_per_hour", normalizeName(createGauge("mongodbatlas.process.oplog.rate", "GiBy/h"), "", false))
+	require.Equal(t, "mongodbatlas_process_db_query_targeting_scanned_per_returned", normalizeName(createGauge("mongodbatlas.process.db.query_targeting.scanned_per_returned", "{scanned}/{returned}"), "", false))


Edge case: what's the expected result if allowedUTF8 == true?

It's translated to scanned_per_returned

bertysentry · 2024-11-04T14:33:32Z

pkg/translator/prometheus/normalize_name_test.go

-	require.Equal(t, "hello_you_2", CleanUpString("hello you 2"))
-	require.Equal(t, "1000", CleanUpString("$1000"))
-	require.Equal(t, "", CleanUpString("*+$^=)"))
+	for _, allowUTF8 := range []bool{true, false} {


Please remove the for/if/else: unnecessary complexity

ArthurSens · 2024-11-04T18:29:20Z

@bertysentry the questions around unit translation are really good ones, I haven't thought about it. This is something that wasn't questioned in the user survey and I don't know what should be the answer here.

Maybe we start by keeping the current behavior, and if needed change this in the future?

@ArthurSens

…-go to the latest release (open-telemetry#36132) #### Description Update github.com/GoogleCloudPlatform/opentelemetry-operations-go to https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/releases/tag/v0.49.0. This unblocks open-telemetry#35904 by moving the prometheus sanitization into contrib. @ArthurSens

fatsheep9146

Please help resolve the conflicts, thanks!

github-actions · 2024-11-29T05:20:50Z

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

ArthurSens · 2024-11-29T16:37:17Z

Before I get back to this PR, is the desired outcome clear to everyone? Or do we want to go back to the drawing board? @dashpole @bertysentry

dashpole · 2024-12-02T15:27:47Z

I don't think the open comments are blocking, as they are mostly orthogonal to this change.

bertysentry · 2024-12-02T16:08:36Z

@ArthurSens The outcome is clear to me, but I'm wondering whether we should align our configuration variables with the way Prometheus implemented the various "translation" methods for OTLP-to-Prom conversion.

If we're all onboard for the combination of flags instead of a single translation_strategy variables, I'm fine with it!

github-actions · 2024-12-17T05:21:30Z

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

bertysentry · 2024-12-17T08:16:37Z

We need this feature, it's not stale.

ArthurSens · 2024-12-17T12:38:15Z

Hey, just an update to interested folks here. Before continuing here, I'm waiting to see how the code will look in Prometheus. I've opened a few PRs over there, but we're still working on how the code will look. Once we have an agreement, I'll propose something here to keep both codebases similar.

We've also discussed removing both libraries from Prometheus and the Collector and moving them to a separate repository during the OTel-Prometheus SIG meetings. Still, it's WIP, and we'll require a design document before committing to the move.

@ArthurSens

…-go to the latest release (open-telemetry#36132) #### Description Update github.com/GoogleCloudPlatform/opentelemetry-operations-go to https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/releases/tag/v0.49.0. This unblocks open-telemetry#35904 by moving the prometheus sanitization into contrib. @ArthurSens

github-actions · 2025-01-01T05:20:58Z

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

ywwg · 2025-01-02T16:45:37Z

@ArthurSens any updates post-holidays?

ArthurSens · 2025-01-03T18:31:03Z

@ArthurSens any updates post-holidays?

I think prometheus/prometheus#15664 is the last big PR to Prometheus' OTLP translation code. After that, we should be able to progress here. My preference is still to create a separate library for the metric name translations.

I assume you haven't had time to look at this separate library, right? We should probably sync about this!

ArthurSens requested review from dashpole and a team as code owners October 21, 2024 19:41

github-actions bot assigned fatsheep9146 Oct 21, 2024

github-actions bot added exporter/prometheus exporter/prometheusremotewrite pkg/translator/prometheus labels Oct 21, 2024

github-actions bot requested review from Aneurysm9, bertysentry and rapphil October 21, 2024 19:42

ArthurSens mentioned this pull request Oct 21, 2024

pkg/translator/prometheus: Allow not normalizing UTF8 characters #35469

Closed

ArthurSens force-pushed the pkg-transl-prom-allowutf8 branch from 690750f to 6a6b470 Compare October 30, 2024 13:40

ArthurSens requested a review from jpkrohling as a code owner October 30, 2024 13:40

github-actions bot added the pkg/translator/loki label Oct 30, 2024

github-actions bot requested review from gouthamve and mar4uk October 30, 2024 13:41

dashpole mentioned this pull request Oct 31, 2024

Remove dependency on prometheus translation package GoogleCloudPlatform/opentelemetry-operations-go#908

Merged

dashpole mentioned this pull request Nov 1, 2024

Update update github.com/GoogleCloudPlatform/opentelemetry-operations-go to the latest release #36132

Merged

ArthurSens force-pushed the pkg-transl-prom-allowutf8 branch from 6a6b470 to dd96ad5 Compare November 2, 2024 22:59

ArthurSens requested a review from jsuereth as a code owner November 2, 2024 22:59

github-actions bot added the exporter/googlemanagedprometheus Google Managed Prometheus exporter label Nov 2, 2024

github-actions bot requested review from aabmass, psx95 and punya November 2, 2024 22:59

ArthurSens force-pushed the pkg-transl-prom-allowutf8 branch from dd96ad5 to 9a1f936 Compare November 3, 2024 17:56

[pkg/translator/prometheus] Add option to keep UTF-8 characters

e5719ea

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

ArthurSens force-pushed the pkg-transl-prom-allowutf8 branch from 9a1f936 to e5719ea Compare November 4, 2024 11:49

bertysentry reviewed Nov 4, 2024

View reviewed changes

dashpole approved these changes Nov 14, 2024

View reviewed changes

fatsheep9146 reviewed Nov 15, 2024

View reviewed changes

github-actions bot added the Stale label Nov 29, 2024

github-actions bot removed the Stale label Nov 30, 2024

github-actions bot added the Stale label Dec 17, 2024

github-actions bot removed the Stale label Dec 18, 2024

github-actions bot added the Stale label Jan 1, 2025

github-actions bot removed the Stale label Jan 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pkg/translator/prometheus] Add option to keep UTF-8 characters #35904

[pkg/translator/prometheus] Add option to keep UTF-8 characters #35904

ArthurSens commented Oct 21, 2024

ArthurSens commented Oct 30, 2024

dashpole commented Oct 30, 2024

ArthurSens commented Oct 30, 2024

dashpole commented Oct 31, 2024

ArthurSens commented Nov 2, 2024

bertysentry Nov 4, 2024

bertysentry Nov 4, 2024

ArthurSens Nov 4, 2024

bertysentry Nov 4, 2024

ArthurSens Nov 4, 2024

bertysentry Nov 4, 2024

ArthurSens commented Nov 4, 2024

fatsheep9146 left a comment

github-actions bot commented Nov 29, 2024

ArthurSens commented Nov 29, 2024

dashpole commented Dec 2, 2024

bertysentry commented Dec 2, 2024

github-actions bot commented Dec 17, 2024

bertysentry commented Dec 17, 2024

ArthurSens commented Dec 17, 2024

github-actions bot commented Jan 1, 2025

ywwg commented Jan 2, 2025

ArthurSens commented Jan 3, 2025

[pkg/translator/prometheus] Add option to keep UTF-8 characters #35904

Are you sure you want to change the base?

[pkg/translator/prometheus] Add option to keep UTF-8 characters #35904

Conversation

ArthurSens commented Oct 21, 2024

Description

Link to tracking issue

ArthurSens commented Oct 30, 2024

dashpole commented Oct 30, 2024

ArthurSens commented Oct 30, 2024

dashpole commented Oct 31, 2024

ArthurSens commented Nov 2, 2024

bertysentry Nov 4, 2024

Choose a reason for hiding this comment

bertysentry Nov 4, 2024

Choose a reason for hiding this comment

ArthurSens Nov 4, 2024

Choose a reason for hiding this comment

bertysentry Nov 4, 2024

Choose a reason for hiding this comment

ArthurSens Nov 4, 2024

Choose a reason for hiding this comment

bertysentry Nov 4, 2024

Choose a reason for hiding this comment

ArthurSens commented Nov 4, 2024

fatsheep9146 left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 29, 2024

ArthurSens commented Nov 29, 2024

dashpole commented Dec 2, 2024

bertysentry commented Dec 2, 2024

github-actions bot commented Dec 17, 2024

bertysentry commented Dec 17, 2024

ArthurSens commented Dec 17, 2024

github-actions bot commented Jan 1, 2025

ywwg commented Jan 2, 2025

ArthurSens commented Jan 3, 2025