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

[exporter/prometheus] add with_resource_constant_labels #67

Conversation

codeboten
Copy link
Contributor

This add support for the following from the spec:

A Prometheus Exporter MAY offer configuration to add
resource attributes as metric attributes. By default,
it MUST NOT add any resource attributes as metric
attributes. The configuration SHOULD allow the user
to select which resource attributes to copy (e.g.
include / exclude or regular expression based). Copied
Resource attributes MUST NOT be excluded from target_info.

See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk_exporters/prometheus.md

@codeboten codeboten requested a review from a team as a code owner January 15, 2024 22:34
@codeboten codeboten force-pushed the codeboten/add-resource-as-constant-labels branch from c2b8dbb to 2841eb8 Compare January 15, 2024 22:39
@@ -125,6 +125,8 @@ meter_provider:
without_units: false
without_type_suffix: false
without_scope_info: false
# Configure Prometheus Exporter to add resource attributes as metrics attributes.
with_resource_as_constant_labels: "service\\.[.]*"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #64 I proposed using the view instrument name wildcard syntax, where ? matches any single character, and * matches any number of characters including none.

The argument being that this avoids having to answer questions about which flavor of regex is used, and whether we can actually accommodate that across languages.

The downside of a simpler wildcard syntax is that you likely need to switch this parameter from a string type to an array of strings, since you can easily run into situations where you can't express the desired set in a single expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, another downside is with a wildcard there isn't a mechanism for exclusions built-in

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of #71?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benefit of #71 is that we get ahead of what I think will be a recurring pattern of wanting to provide a way in file config to specify an allow list or disallow list with pattern matching. Regex comes with the complexity of specifying the version of regular expressions, and if a language doesn't support the specification, we'll have worse interoperability because regexes will need to be adjusted for an individual language's requirements. Glob pattern matching is straight forward to implement and gets most of there of what you want to do with regex.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts on how one would use #71 to include everything except something that matches? I'm thinking specifically in the collector where someone may want to include all resource attributes except for maybe one of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the simplicity of wildcards better, so long as we retain the ability to have an include or exclude match, then that would solve my use-case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Various components in the collector support the following for include/exclude rules:

include:
    - /var/log/pods/default_*/*/*.log
exclude: []

@jack-berg if you're ok with this, it can be added to #71

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that sounds great. Do you want to reflect that in this PR and merge? Then I can update #71 in a followup to reflect that syntax we use here.

Copy link
Contributor Author

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codeboten codeboten force-pushed the codeboten/add-resource-as-constant-labels branch from 2841eb8 to c92ff52 Compare April 24, 2024 19:59
@codeboten codeboten changed the title [exporter/prometheus] add with_resource_as_constant_labels [exporter/prometheus] add with_resource_constant_labels Apr 24, 2024
@codeboten codeboten force-pushed the codeboten/add-resource-as-constant-labels branch from c92ff52 to 48526e5 Compare May 8, 2024 17:37
examples/kitchen-sink.yaml Show resolved Hide resolved
This add support for the following from the spec:

> A Prometheus Exporter MAY offer configuration to add
> resource attributes as metric attributes. By default,
> it MUST NOT add any resource attributes as metric
> attributes. The configuration SHOULD allow the user
> to select which resource attributes to copy (e.g.
> include / exclude or regular expression based). Copied
> Resource attributes MUST NOT be excluded from target_info.

Signed-off-by: Alex Boten <aboten@lightstep.com>
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten codeboten force-pushed the codeboten/add-resource-as-constant-labels branch from 41b8e1f to 3a812cb Compare May 8, 2024 18:34
@codeboten codeboten merged commit a87ec9e into open-telemetry:main May 8, 2024
2 checks passed
@codeboten codeboten deleted the codeboten/add-resource-as-constant-labels branch May 8, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants