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

Clarify that attribute processor does not drop signals #5178

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

ptodev
Copy link
Contributor

@ptodev ptodev commented Sep 13, 2023

PR Description

There has been some confusion regarding the attributes processor. It contains an exclude block, and people sometimes get confused that this means the data will be excluded in downstream components. This is not the case.

I mention the tail sampling processor as a processor which can do this. However, it only works for traces. It would have been nice to also point to similar processors for logs and metrics, but I don't think we have such components in Flow at the moment. I think the otel components for dropping metrics and logs are metricstransform processor and probabilisticsampler processor.

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@@ -164,6 +164,10 @@ For example, adding a `span_names` filter could cause the component to error if

The `exclude` block provides an option to exclude data from being fed into the [action] blocks based on the properties of a span, log, or metric records.
Copy link
Member

Choose a reason for hiding this comment

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

Is this true?

Suggested change
The `exclude` block provides an option to exclude data from being fed into the [action] blocks based on the properties of a span, log, or metric records.
The `exclude` block provides an option to exclude data from being fed into the [action] blocks based on the properties of a span, log, or metric records. Excluded data will be forwarded as-is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is true, but I will add it to the new block that I'm adding now, since clarifying this is the whole purpose of the block. I'm also not sure if readers would know what is meant by "forwarded". I'll change this:

Signals excluded by the exclude block will still be propagated to downstream components.

To:

Signals excluded by the exclude block will still be propagated to downstream components as-is.

Comment on lines 167 to 169
> **NOTE**: Signals excluded by the `exclude` block will still be propagated to downstream components.
> If you would like to not propagate certain signals to downstream components,
> consider a processor such as [otelcol.processor.tail_sampling](../otelcol.processor.tail_sampling/).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> **NOTE**: Signals excluded by the `exclude` block will still be propagated to downstream components.
> If you would like to not propagate certain signals to downstream components,
> consider a processor such as [otelcol.processor.tail_sampling](../otelcol.processor.tail_sampling/).
{{% admonition type="note" %}}
Signals excluded by the `exclude` block are still be sent to downstream components.
To prevent specific signals from being sent to downstream components, you can use a processor
such as [otelcol.processor.tail_sampling](../otelcol.processor.tail_sampling/).
{{% /admonition %}}

Better to use the admonition shortcode for notes where we can.

Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

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

@ptodev Docs look ok. Are there more changes pending?

@tpaschalis tpaschalis enabled auto-merge (squash) September 18, 2023 07:15
@tpaschalis tpaschalis merged commit c349989 into main Sep 18, 2023
6 checks passed
@tpaschalis tpaschalis deleted the clarify-attributes-processor branch September 18, 2023 07:18
@ptodev
Copy link
Contributor Author

ptodev commented Sep 18, 2023

@ptodev Docs look ok. Are there more changes pending?

Nope! This is all I wanted to add.

@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Sep 18, 2023
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 22, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants