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

[attributesprocessor] The resource part of metrics include filtering config doesn't work #30642

Closed
chenlujjj opened this issue Jan 17, 2024 · 15 comments
Labels
bug Something isn't working processor/attributes Attributes processor processor/transform Transform processor

Comments

@chenlujjj
Copy link
Contributor

Component(s)

processor/attributes

What happened?

Description

As you can see in the attached collector config file, I tried to use the attributes processor to add labels for the metric node_disk_write_time_seconds_total only when device=disk0

NOTE: this is just a minimum example to reproduce the issue.

Steps to Reproduce

Expected Result

Actual Result

All the metrics node_disk_write_time_seconds_total even with device!=disk0 were updated as well.

Collector version

0.79.0

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

receivers:
    prometheus:
      config:
        scrape_configs:
          - job_name: "node_exporter"
            scrape_interval: 10s
            static_configs:
              - targets: ['0.0.0.0:9100']

exporters:
    file/metrics:
      path: /tmp/metrics

processors:
    batch:
    attributes/1:
      include:
        resources:
          - key: device
            value: disk0
        metric_names: [node_disk_write_time_seconds_total]
        match_type: strict
      actions:
        - key: name
          action: upsert
          value: zhangsan

extensions:
    health_check:
      endpoint: 0.0.0.0:13133
    pprof:
      endpoint: :1888
    zpages:
      endpoint: :55679

service:
    extensions: [pprof, zpages, health_check]
    telemetry:
      metrics:
        address: ":8888"
    pipelines:
      metrics:
        receivers: [prometheus]
        processors: [batch, attributes/1]
        exporters: [logging, file/metrics]

Log output

No response

Additional context

In the source code:

The ResourceAttributes of MatchProperties doesn't take effect at all. Only the metric names are used to filter.

@chenlujjj chenlujjj added bug Something isn't working needs triage New item requiring triage labels Jan 17, 2024
@github-actions github-actions bot added the processor/attributes Attributes processor label Jan 17, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@chenlujjj
Copy link
Contributor Author

The readme doc says: "For metrics, one of metric_names or resources must be specified with a valid non-empty value for a valid configuration." So I think the resources can be used to filter as well.

@chenlujjj
Copy link
Contributor Author

And when I just specified the resources like:

attributes/1:
      include:
        resources:
          - key: device
            value: disk0
     actions:
        - key: name
          action: upsert
          value: zhangsan

It will update labels for all metrics ...
The cause is at this line: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.79.0/internal/filter/filtermetric/filtermetric.go#L47

@crobert-1
Copy link
Member

To clarify your minimum reproducible config file, you're saying that with this config all metrics named node_disk_write_time_seconds_total are coming through, even if their device resource isn't disk0, is that correct?

If that's the case, could you post some example metrics from your logging output?

@chenlujjj
Copy link
Contributor Author

hi, @crobert-1, please see the attached file for the full list of node exporter metrics.
node_metrics.json

image

@crobert-1
Copy link
Member

Okay, I'm seeing two main issues here.

  1. The filter expression only accounts for the metric name from what I'm seeing, it drops the resource attribute filter from the check. This is what is causing part of your issue. Adding a reference to NewResourceSkipExprBridge to properly account for resource attribute filters would resolve this.
  2. The attributes you're looking at are datapoint attributes, not resource attributes. However, this would still fail due to the bug referenced in my first point, attributes aren't properly accounted for either from what I can tell.

There is another more supported way to do this though, using the transform processor. If you use the datapoint context you can use the set function to add a key to your attributes map when specific conditions are met, as you're attempting to do here.

@crobert-1 crobert-1 added processor/transform Transform processor and removed needs triage New item requiring triage labels Jan 18, 2024
Copy link
Contributor

Pinging code owners for processor/transform: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@chenlujjj
Copy link
Contributor Author

Hi @crobert-1 , thanks for your reply.
If I understand correctly, the device filter specified in the config above will not work even if the resource attribute filter is added to the attributes processor properly, because the device is a datapoint attribute, not resource attribute, is that right?

@chenlujjj
Copy link
Contributor Author

And from the code here, the attributesprocessor processes the datapoint attribute, not resource attribute, too.

I'll give a try to transform processor, thanks.

@crobert-1
Copy link
Member

If I understand correctly, the device filter specified in the config above will not work even if the resource attribute filter is added to the attributes processor properly, because the device is a datapoint attribute, not resource attribute, is that right?

That's correct, even if this bug is fixed the filter you've shared won't work with your data.

And from the code here, the attributesprocessor processes the datapoint attribute, not resource attribute, too.

Yes, the attributes processor only supports operations on datapoint attributes for metrics, it does not support operations on resource attributes.

@chenlujjj
Copy link
Contributor Author

Thanks for your confirmation :)

@TylerHelmuth
Copy link
Member

In the transformprocessor it would look like

transform:
  error_mode: ignore
  metric_statements:
    - context: datapoint
      statements:
        - set(name, "zhangsan") where metric.name == "node_disk_write_time_seconds_total" and resource.attributes["device"] == "disk0"

@TylerHelmuth
Copy link
Member

@chenlujjj I am going to close this issue for now since that transformprocessor config solves the use case you wanted. Please ping me if it doesnt work and I'll reopen.

@chenlujjj
Copy link
Contributor Author

Thank you @TylerHelmuth
After trial, I found the right config of transform processor for this case is:

transform:
      error_mode: ignore
      metric_statements:
        - context: datapoint
          statements:
            - set(attributes["name"], "zhangsan") where metric.name == "node_disk_write_time_seconds_total" and attributes["device"] == "disk0"

@TylerHelmuth
Copy link
Member

You're right, good correction!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working processor/attributes Attributes processor processor/transform Transform processor
Projects
None yet
Development

No branches or pull requests

3 participants