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

Supporting Key and a List of Values in the Attribute Processor #1935

Closed
isgoutham opened this issue Oct 12, 2020 · 12 comments
Closed

Supporting Key and a List of Values in the Attribute Processor #1935

isgoutham opened this issue Oct 12, 2020 · 12 comments

Comments

@isgoutham
Copy link

Support for key and list based values in the attribute processor.

Use case: To support list of homogeneous values under a single key.

Example Configuration(otel-collector-config):

  attributes/priority_sampling:
    include:
      match_type: strict
      attributes:
        - key: http.url
          value: 
            - 'http://productcatalogservice/GetProducts'
            - 'http://recommendationservice/GetRecommendations'
            - 'http://cartservice/GetCart'
        
    actions:
      - key: sampling.priority
        value: 1

All the values under http.url should have a sampling.priority value as 1. Instead, current implementation of OTEL is not supporting this feature and triggers a bug.

Steps to produce:

  1. Add otel collector config
  2. Execute the demo app

Config file: - otel-collector-config.yaml

receivers:
  opencensus:

exporters:
  prometheus:
    endpoint: "0.0.0.0:8889"
    namespace: promexample
    const_labels:
      label1: value1
  logging:

  zipkin:
    endpoint: "http://zipkin-all-in-one:9411/api/v2/spans"
    format: proto

  jaeger:
    endpoint: jaeger-all-in-one:14250
    insecure: true

processors:
  batch:
  queued_retry:
  attributes/priority_sampling:
    include:
      match_type: strict
      attributes:
        - key: http.url
          value: 
            - 'http://productcatalogservice/GetProducts'
            - 'http://recommendationservice/GetRecommendations'
            - 'http://cartservice/GetCart'
        
    actions:
      - key: sampling.priority
        value: 1
        action: upsert

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

service:
  extensions: [pprof, zpages, health_check]
  pipelines:
    traces:
      receivers: [opencensus]
      processors: [batch, queued_retry, attributes/priority_sampling]
      exporters: [logging, zipkin, jaeger]
    metrics:
      receivers: [opencensus]
      processors: [batch]
      exporters: [logging,prometheus]

Error: cannot setup pipelines: cannot build pipelines: error creating processor "attributes/priority_sampling" in pipeline "traces": error creating attribute filters: error unsupported value type "[]interface {}"

image

image

PR will be added for supporting and resolving the same. This will enable list of values (homogeneous) under a key for attribute processors.

@isgoutham
Copy link
Author

isgoutham commented Oct 12, 2020

PR - #1936

Above PR adds support for list of values under a key in the attribute processor

Eg screens:

image
image

@isgoutham isgoutham changed the title Support for Key and a List of Values in the Attribute Processor Supporting Key and a List of Values in the Attribute Processor Oct 12, 2020
@isgoutham
Copy link
Author

@tigrannajaryan could you please check/review this

@isgoutham
Copy link
Author

@tigrannajaryan gentle reminder

@tigrannajaryan
Copy link
Member

Use case: To support list of homogeneous values under a single key.

Sounds good to me. Doesn't have to be homogeneous, we can support heterogeneous arrays too.

@isgoutham
Copy link
Author

Thank you @tigrannajaryan

yes we can support heterogeneous arrays. I was under the impression that a single key can carry homogeneous values (like http.url with only string values).

if everything is fine, could you please review/approve? few of our cases are have been long ceased for this change

@tigrannajaryan
Copy link
Member

@isgoutham I misunderstood your proposal initially.

You are suggesting that when a list of values is specified in the configuration of the attributes processor then it should match an attribute with a value that is equal to any of the values listed in the config. In other words this is a search of an attribute value in the array of values specified in the config.

So, for example let's say we have the following config:

  attributes/priority_sampling:
    include:
      match_type: strict
      attributes:
        - key: http.url
          value: 
            - 'http://productcatalogservice/GetProducts'
            - 'http://recommendationservice/GetRecommendations'
            - 'http://cartservice/GetCart'

This would match a span with attribute http.url equal to string "http://productcatalogservice/GetProducts".

This is an unexpected and surprising behavior. match_type: strict means that we are looking for the exact value as it is specified in the config, not for one of the values. I would expect that this does a full array comparison, not an array element search. Arrays are valid attribute values and we should support. I would expect that with the above configuration we would match a span with an attribute http.url equal exactly to an array ["http://productcatalogservice/GetProducts", "http://recommendationservice/GetRecommendations", "http://cartservice/GetCart"].

What you are looking for can be achieved today by using multiple attributes processors:

attributes/priority_sampling1:
    include:
      match_type: strict
      attributes:
        - key: http.url
          value: 'http://productcatalogservice/GetProducts'
    actions:
       ...
attributes/priority_sampling2:
    include:
      match_type: strict
      attributes:
        - key: http.url
          value: 'http://productcatalogservice/GetProducts'
    actions:
       ...
attributes/priority_sampling3:
    include:
      match_type: strict
      attributes:
        - key: http.url
          value: 'http://cartservice/GetCart'
    actions:
       ...

Unfortunately this is quite verbose so perhaps a different way is necessary to do what you need. However, I do not think we should do what you suggest because it is both unexpected behavior and also closes the door for proper support for attribute values.

As an alternate approach I would look into supporting match_type: expr to allow complicated expressions, including matching one of the values in an array. We already have a proposal for expression matching for metrics here #1940 and can add a similar support for span attributes.

Another possibility would be to support yet another match_type that allows array element searches (but my preference would be to see how we can fit this into match_type: expr).

@tigrannajaryan
Copy link
Member

One more alternate that works today is to use regexp with alternates:

attributes/priority_sampling:
    include:
      match_type: regexp
      attributes:
        - key: http.url
          value: http://cartservice/GetCart|http://productcatalogservice/GetProducts
    actions:
       ...

@asldevi
Copy link

asldevi commented Oct 28, 2020

looks like regexp support for attribute processor is added in v0.13.0. Thanks for your pointer @tigrannajaryan

@isgoutham
Copy link
Author

isgoutham commented Oct 28, 2020

@tigrannajaryan thanks a ton for very neat and elaborative explaination.
Tried with below config, and it's working as expected. Can close the issue & PR.

    include:
      match_type: regexp
      attributes:
        - key: http.url
          value: http://productcatalogservice/GetProducts%7Chttp://recommendationservice/GetRecommendations%7Chttp://cartservice/GetCart
    actions:
      - key: sampling.priority
        value: 1
        action: upsert```

@isgoutham
Copy link
Author

Which approach would you suggest for these complex span attributes ?

under match_type: expr or under match_type: regex ?

@tigrannajaryan
Copy link
Member

Which approach would you suggest for these complex span attributes ?

I am not sure I understand the question. Which complex attributes?

@isgoutham
Copy link
Author

Multiple values under a key.

Asking about this

  • Another possibility would be to support yet another match_type that allows array element searches (but my preference would be to see how we can fit this into match_type: expr)

tigrannajaryan pushed a commit that referenced this issue Dec 1, 2020
Documentation for processors states:

> Only match_type=strict is allowed if "attributes" are specified.

but this restriction was removed in https://github.com/open-telemetry/opentelemetry-collector/pull/928/files#diff-4548db28578c2ac90e2b277f24654cfa24fd0f99d854e0fcc4b50871c0b529caL166-R198, and so this doc appears to be outdated.

**Testing:**

I did not test this, but others (including @tigrannajaryan) have: #1935 (comment)

[btw, in case you're curious my interest in this, we are implementing a subset of this behavior at the java agent layer, as we aren't using otel collector. So far only in our vendor distro, though happy to move it to otel javaagent if/when others are interested.]
@github-actions github-actions bot added the Stale label Jan 14, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants