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

Prometheus - Allow to split attribute values by a delimiter into new metric series #5136

Closed
StarpTech opened this issue Apr 3, 2024 · 11 comments · Fixed by #5159
Closed
Labels
enhancement New feature or request pkg:exporter:prometheus Related to the Prometheus exporter package

Comments

@StarpTech
Copy link

StarpTech commented Apr 3, 2024

Problem Statement

In OpenTelemetry (OTEL), assigning a list of values to an attribute isn't directly supported. However, there's a helper method called StringSlice which converts the value into a JSON array. It's then up to the consumer to parse it during ingestion.

On the other hand, Prometheus doesn't support parsing JSON. Therefore, for optimal compatibility, the data must be prepared and ready at scrape time.

Proposed Solution

I suggest introducing an additional feature called .WithMultiValues(",") to the Prometheus exporter. This feature would enable the splitting of metric labels by a specified delimiter into different metric series. This approach would allow us to combine the best aspects of both worlds, providing greater flexibility and compatibility. I'm not bound strictly to the delimiter aspect; alternatively, we could also reach a consensus on the output of StringSlice.

This feature is easy to add because it only affects the output of prometheus. We would need to iterate over all metrics attributes, split the value by the delimiter and duplicate the metric by the new label value.

Alternatives

It seems that achieving this functionality solely through the relabel_configs of Prometheus is not feasible, as it lacks the necessary flexibility. Additionally, adding a preprocessing step to all Prometheus platforms may not always be practical or possible.

Therefore, introducing the proposed .WithMultiValues(",") option directly within the Prometheus exporter would offer a more straightforward and universally applicable solution to splitting metric labels by a delimiter into different metric series. This approach would alleviate the need for additional preprocessing steps and ensure compatibility across various Prometheus platforms.

@StarpTech StarpTech added the enhancement New feature or request label Apr 3, 2024
@dmathieu
Copy link
Member

dmathieu commented Apr 3, 2024

Note that the string representation of StringSlice is not JSON. It's the Go string representation of []string{}.
https://github.com/open-telemetry/opentelemetry-go/blob/v1.24.0/attribute/value.go#L253
https://go.dev/play/p/J-OHRMixzzZ

This looks like an invalid transformation of attributers in the prometheus exporter, which simply uses Emit().
https://github.com/open-telemetry/opentelemetry-go/blob/main/exporters/prometheus/exporter.go#L308

We could possibly add the WithMultiValues option, but I think this should be fixed with a sane default (probably merging the values into a comma-separated string, as your option suggests), and the option should only be there if folks want to change the default.

@StarpTech
Copy link
Author

Note that the string representation of StringSlice is not JSON. It's the Go string representation of []string{}.

Thanks for the pointers. This is even more confusing because the stored data would be language specific.

We could possibly add the WithMultiValues option, but I think this should be fixed with a sane default (probably merging the values into a comma-separated string, as your option suggests), and the option should only be there if folks want to change the default.

Sounds good.

@StarpTech
Copy link
Author

Would you accept a PR?

@dmathieu
Copy link
Member

dmathieu commented Apr 3, 2024

As you mention above, the current representation is go-specific. So we should be adding clarification in the specification.
I have opened an issue about that: open-telemetry/opentelemetry-specification#3982

@StarpTech
Copy link
Author

I had the same idea. Thanks for opening an issue.

@dashpole dashpole added the pkg:exporter:prometheus Related to the Prometheus exporter package label Apr 4, 2024
@dashpole
Copy link
Contributor

dashpole commented Apr 4, 2024

IIUC, the options we are considering are:

  • Current: "[B C D]"
  • Specified: "[\"B\",\"C\",\"D\"]"
  • Proposed: "B,C,D"

@StarpTech Does prometheus support parsing the proposed format? How would you do that?

@StarpTech
Copy link
Author

Does prometheus support parsing the proposed format? How would you do that?

No, the prometheus exporter would need to split "B,C,D" and create multiple metric series, each with the key=values as new label=value set

// OTEL
Metric: mymetric
attr1="B,C,D"

// Prometheus
mymetric{attr1="B"}
mymetric{attr1="C"}
mymetric{attr1="D"}

@dashpole
Copy link
Contributor

dashpole commented Apr 4, 2024

Won't that break totals? E.g.

# TYPE mymetric counter
mymetric{attr1="B,C,D"} 100

gives 100 as the total.

# TYPE mymetric counter
mymetric{attr1="B"} 100
mymetric{attr1="C"} 100
mymetric{attr1="D"} 100

gives 300 as the total.

@StarpTech
Copy link
Author

StarpTech commented Apr 4, 2024

You're correct, and upon reconsideration, it seems I overlooked an important aspect here. To replicate the same behavior, I simply need to emit the metrics three times, each with its own attribute values. It is funny though, that we all had a different understanding 😄 Thank you!

c.Add(ctx, 1, attribute.Key("wg.subgraph.error.extended_code").String("B"))
c.Add(ctx, 1, attribute.Key("wg.subgraph.error.extended_code").String("C"))
c.Add(ctx, 1, attribute.Key("wg.subgraph.error.extended_code").String("D"))

I don't know why I didn't test it before. This is the result

Search for wg_subgraph_error_extended_code

 HELP router_http_requests_error_total Total number of failed request
# TYPE router_http_requests_error_total counter
router_http_requests_error_total{http_status_code="200",otel_scope_name="cosmo.router.prometheus",otel_scope_version="0.0.1",wg_client_name="unknown",wg_client_version="missing",wg_component_name="engine-loader",wg_federated_graph_id="726f79c8-68a7-4fef-af9b-a318d4d938d3",wg_operation_name="",wg_operation_protocol="http",wg_operation_type="query",wg_router_cluster_name="",wg_router_config_version="8e9ce471-4d11-4690-b0bc-a53bf51b5acc",wg_router_version="dev",wg_subgraph_error_extended_code="unknown",wg_subgraph_id="3866af85-6f32-42a4-a0bc-da1ea8e73f2f",wg_subgraph_name="family"} 2
router_http_requests_error_total{http_status_code="200",otel_scope_name="cosmo.router.prometheus",otel_scope_version="0.0.1",wg_client_name="unknown",wg_client_version="missing",wg_component_name="engine-loader",wg_federated_graph_id="726f79c8-68a7-4fef-af9b-a318d4d938d3",wg_operation_name="",wg_operation_protocol="http",wg_operation_type="query",wg_router_cluster_name="",wg_router_config_version="8e9ce471-4d11-4690-b0bc-a53bf51b5acc",wg_router_version="dev",wg_subgraph_error_extended_code="unknown",wg_subgraph_id="42dad70b-04ac-480c-8850-5afc5809cb38",wg_subgraph_name="employees"} 2
router_http_requests_error_total{http_status_code="200",otel_scope_name="cosmo.router.prometheus",otel_scope_version="0.0.1",wg_client_name="unknown",wg_client_version="missing",wg_component_name="engine-loader",wg_federated_graph_id="726f79c8-68a7-4fef-af9b-a318d4d938d3",wg_operation_name="",wg_operation_protocol="http",wg_operation_type="query",wg_router_cluster_name="",wg_router_config_version="8e9ce471-4d11-4690-b0bc-a53bf51b5acc",wg_router_version="dev",wg_subgraph_error_extended_code="unknown",wg_subgraph_id="a5b74628-c040-4669-9655-d8c895664eed",wg_subgraph_name="products"} 3
router_http_requests_error_total{http_status_code="200",otel_scope_name="cosmo.router.prometheus",otel_scope_version="0.0.1",wg_client_name="unknown",wg_client_version="missing",wg_component_name="engine-loader",wg_federated_graph_id="726f79c8-68a7-4fef-af9b-a318d4d938d3",wg_operation_name="",wg_operation_protocol="http",wg_operation_type="query",wg_router_cluster_name="",wg_router_config_version="8e9ce471-4d11-4690-b0bc-a53bf51b5acc",wg_router_version="dev",wg_subgraph_error_extended_code="unknown",wg_subgraph_id="c84a059f-4ae8-4835-926a-76e9a337ccb0",wg_subgraph_name="hobbies"} 1
router_http_requests_error_total{http_status_code="200",otel_scope_name="cosmo.router.prometheus",otel_scope_version="0.0.1",wg_client_name="unknown",wg_client_version="missing",wg_component_name="engine-loader",wg_federated_graph_id="726f79c8-68a7-4fef-af9b-a318d4d938d3",wg_operation_name="",wg_operation_protocol="http",wg_operation_type="query",wg_router_cluster_name="",wg_router_config_version="8e9ce471-4d11-4690-b0bc-a53bf51b5acc",wg_router_version="dev",wg_subgraph_error_extended_code="unknown2",wg_subgraph_id="3866af85-6f32-42a4-a0bc-da1ea8e73f2f",wg_subgraph_name="family"} 2
router_http_requests_error_total{http_status_code="200",otel_scope_name="cosmo.router.prometheus",otel_scope_version="0.0.1",wg_client_name="unknown",wg_client_version="missing",wg_component_name="engine-loader",wg_federated_graph_id="726f79c8-68a7-4fef-af9b-a318d4d938d3",wg_operation_name="",wg_operation_protocol="http",wg_operation_type="query",wg_router_cluster_name="",wg_router_config_version="8e9ce471-4d11-4690-b0bc-a53bf51b5acc",wg_router_version="dev",wg_subgraph_error_extended_code="unknown2",wg_subgraph_id="42dad70b-04ac-480c-8850-5afc5809cb38",wg_subgraph_name="employees"} 2
router_http_requests_error_total{http_status_code="200",otel_scope_name="cosmo.router.prometheus",otel_scope_version="0.0.1",wg_client_name="unknown",wg_client_version="missing",wg_component_name="engine-loader",wg_federated_graph_id="726f79c8-68a7-4fef-af9b-a318d4d938d3",wg_operation_name="",wg_operation_protocol="http",wg_operation_type="query",wg_router_cluster_name="",wg_router_config_version="8e9ce471-4d11-4690-b0bc-a53bf51b5acc",wg_router_version="dev",wg_subgraph_error_extended_code="unknown2",wg_subgraph_id="a5b74628-c040-4669-9655-d8c895664eed",wg_subgraph_name="products"} 3
router_http_requests_error_total{http_status_code="200",otel_scope_name="cosmo.router.prometheus",otel_scope_version="0.0.1",wg_client_name="unknown",wg_client_version="missing",wg_component_name="engine-loader",wg_federated_graph_id="726f79c8-68a7-4fef-af9b-a318d4d938d3",wg_operation_name="",wg_operation_protocol="http",wg_operation_type="query",wg_router_cluster_name="",wg_router_config_version="8e9ce471-4d11-4690-b0bc-a53bf51b5acc",wg_router_version="dev",wg_subgraph_error_extended_code="unknown2",wg_subgraph_id="c84a059f-4ae8-4835-926a-76e9a337ccb0",wg_subgraph_name="hobbies"} 1

@dmathieu
Copy link
Member

dmathieu commented Apr 4, 2024

I suppose the question remains on how slice attributes are useful for prometheus metrics, or if they should just never be used (and are there purely for historical/specification reasons)

@dashpole
Copy link
Contributor

dashpole commented Apr 4, 2024

I lean towards "they should never be used". That was the conclusion we came to when discussing potentially adding maps to the common attribute type.

This issue did alert me to the fact that our encoding of attributes doesn't follow the spec, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pkg:exporter:prometheus Related to the Prometheus exporter package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants