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

[tqlotel] Add priority keys parameter to the limit function. #12997

Merged
merged 2 commits into from
Sep 7, 2022

Conversation

kovrus
Copy link
Member

@kovrus kovrus commented Aug 8, 2022

Description:

Add ability to specify attributes that aren't allowed to be dropped during limiting. Resolves #9734.

Link to tracking Issue:
#9734

Testing:

  • Unit tests
  • Running Otel collector locally with following configurations:
receivers:
  otlp:
    protocols:
      grpc:

exporters:
  logging:

processors:
  transform:
    traces:
      queries:
        - limit(attributes, 1, "http.url")
service:
  pipelines:
    traces:
      receivers: [otlp]
      processors: [transform]
      exporters: [logging]

and testing with

trace.set_tracer_provider(TracerProvider())
tracer = trace.get_tracer(__name__)
otlp_exporter = OTLPSpanExporter()
span_processor = BatchSpanProcessor(otlp_exporter)
trace.get_tracer_provider().add_span_processor(span_processor)

with tracer.start_as_current_span("foo") as span:
    span.set_attributes({
        SpanAttributes.HTTP_HOST: "",
        SpanAttributes.HTTP_METHOD: "",
        SpanAttributes.HTTP_STATUS_CODE: "",
        SpanAttributes.HTTP_TARGET: "",
        SpanAttributes.HTTP_CLIENT_IP: "",
        SpanAttributes.HTTP_URL: ""
    })

Documentation:
Updated:

  • pkg/telemetryquerylanguage/functions/tqlotel/README.md
  • processor/transformprocessor/README.md

@kovrus kovrus marked this pull request as ready for review August 8, 2022 12:43
@kovrus kovrus requested a review from a team August 8, 2022 12:43
@TylerHelmuth
Copy link
Member

/cc @kentquirk

@kovrus
Copy link
Member Author

kovrus commented Aug 10, 2022

@bogdandrutu @TylerHelmuth can you please review the PR?

@kovrus kovrus force-pushed the tqlotel/limit-filtering branch 2 times, most recently from 25bfc95 to a106878 Compare August 16, 2022 12:37
unreleased/tql-limit-func-priority-keys.yaml Outdated Show resolved Hide resolved
unreleased/tql-limit-func-priority-keys.yaml Outdated Show resolved Hide resolved
unreleased/tql-limit-func-priority-keys.yaml Outdated Show resolved Hide resolved
Copy link
Member

@kentquirk kentquirk left a comment

Choose a reason for hiding this comment

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

This looks good.

I made one minor documentation suggestion.

@TylerHelmuth TylerHelmuth added the ready to merge Code review completed; ready to merge by maintainers label Sep 5, 2022
Comment on lines +109 to +110
`priority_keys` is a list of strings of attribute keys that won't be dropped during limiting.

Copy link
Member

Choose a reason for hiding this comment

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

Is not a list is a variadic argument. I mean this means we cannot add arguments after and we don't need to use join/concat/etc.

Copy link
Member

Choose a reason for hiding this comment

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

@TylerHelmuth how do we call these arguments?

Copy link
Member

Choose a reason for hiding this comment

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

It is a limitation of the grammar at the moment. The grammar only supports 1 list parameter and it must be the last. Yes it is technically variadic, but I believe the docs currently call it a list. There is open issue to add full list capabilities.

@bogdandrutu bogdandrutu merged commit 84ec432 into open-telemetry:main Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[processor/transform] Add ability to specify attributes that aren't allowed to be dropped during limiting
4 participants