feat(comp/otelcol): component to filter OTEL telemetry #5562

hainenber · 2023-10-22T11:39:17Z

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

CHANGELOG.md updated
Documentation added
Tests updated
Config converters updated

ptodev

Thank you very much for this contribution! This has been a much-requested feature. I added a few comments for things we will need to change prior to merging.

component/otelcol/processor/filter/filter.go

docs/sources/flow/reference/components/otelcol.processor.filter.md

component/otelcol/processor/filter/filter.go

docs/sources/flow/reference/components/otelcol.processor.filter.md

hainenber · 2023-10-23T15:46:35Z

Appreciate your review effort @ptodev! Other comments totally makes sense to me but there are three with similar concept so I'd like to discuss jointly.

I saw that the config schema with include, exclude and alike are at brink of being deprecated, as dictated in the ref. Should we also implement it for a brand new Flow component? I feel like it could be technical baggage and we'll revisit for the seemingly inevitable removal.

clayton-cornell

Some doc suggestions and questions

docs/sources/flow/reference/components/otelcol.processor.filter.md

clayton-cornell · 2023-10-24T22:46:38Z

docs/sources/flow/reference/components/otelcol.processor.filter.md

+{{% admonition type="warning" %}}
+Exercise caution when using `otelcol.processor.filter`:
+
+- Make sure you understand the look of the incoming data and test the configuration thoroughly. 


What is meant by "understand the look"? The meaning isn't clear and I'm not sure what to suggest here.

It should be something akin to understanding the data's schema/format. Let me wordsmith this one

Finish in latest commit. PTAL when you have time. Thanks!

docs/sources/flow/reference/components/otelcol.processor.filter.md

ptodev · 2023-10-26T23:13:45Z

Appreciate your review effort @ptodev! Other comments totally makes sense to me but there are three with similar concept so I'd like to discuss jointly.

I saw that the config schema with include, exclude and alike are at brink of being deprecated, as dictated in the ref. Should we also implement it for a brand new Flow component? I feel like it could be technical baggage and we'll revisit for the seemingly inevitable removal.

@hainenber Thank you for pointing this out! I did not realise it. In that case I agree that we can just have the OTTL parameters. We don't need to include these other ones which will be deprecated.

ptodev · 2023-10-26T23:16:01Z

@hainenber Would you mind addressing @clayton-cornell's comments please, and rebasing the branch? I will then take a final look and will likely merge it. Thanks again very much for your work on this component!

Signed-off-by: hainenber <dotronghai96@gmail.com>

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Signed-off-by: hainenber <dotronghai96@gmail.com>

…r.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

…r.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

Signed-off-by: hainenber <dotronghai96@gmail.com>

hainenber · 2023-10-28T06:25:46Z

@hainenber Would you mind addressing @clayton-cornell's comments please, and rebasing the branch? I will then take a final look and will likely merge it. Thanks again very much for your work on this component!

@ptodev I've addressed Clayton's comments and rebased the branch. PTAL when you're back. Thanks!

ptodev

Thank you, it looks good! I left a few minor comments, but the only thing which really needs to be fixed is just the non-working unit test.

component/otelcol/processor/filter/filter_test.go

ptodev · 2023-10-30T10:35:57Z

component/otelcol/processor/filter/types.go

Could you please also move the convert() functions for traceConfig, metricConfig, and logConfig to this file (types.go)?

docs/sources/flow/reference/components/otelcol.processor.filter.md

ptodev · 2023-10-30T11:20:44Z

docs/sources/flow/reference/components/otelcol.processor.filter.md

+{{% admonition type="note" %}}
+Some characters inside River strings [need to be escaped][river-strings] with a `\` character.
+For example, the OTTL statement `attributes["grpc"] == true` 
+is written in River as `attributes[\"grpc\"] == true`.
+
+[river-strings]: {{< relref "../../config-language/expressions/types_and_values.md/#strings" >}}
+{{% /admonition %}}


The next release of the agent will support raw strings, so I updated some of the documentation in #5636. I'm happy to update the otelcol.processor.filter docs after your PR is merged though. There's no need to mention raw strings in your PR.

I've added reference to use River's raw string. It's good by now?

Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Signed-off-by: hainenber <dotronghai96@gmail.com>

hainenber · 2023-10-30T17:48:18Z

Thank you for another review! I've accepted most of your suggested changes and moved the convert() to types.go file. PTAL if it's OK by now.

* feat(comp/otelcol): component to filter OTEL telemetry --------- Signed-off-by: hainenber <dotronghai96@gmail.com> Co-authored-by: Paulin Todev <paulin.todev@gmail.com> Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

hainenber marked this pull request as ready for review October 22, 2023 15:36

hainenber requested review from a team and clayton-cornell as code owners October 22, 2023 15:36

ptodev requested changes Oct 23, 2023

View reviewed changes

clayton-cornell reviewed Oct 24, 2023

View reviewed changes

hainenber and others added 21 commits October 28, 2023 13:22

feat(comp/otelcol): component to filter OTEL telemetry

e3aa250

Signed-off-by: hainenber <dotronghai96@gmail.com>

doc(ref): add doc for otelcol.processor.filter

3b5e418

Signed-off-by: hainenber <dotronghai96@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

1f2325e

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

0640d33

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

6912f9e

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

3108a5a

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

f5978a1

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

f83dae0

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

6b65a76

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

50c261b

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

8bbb9fa

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

refactor(otelcol/filter): move non-arg types to own file

b6cdef8

Signed-off-by: hainenber <dotronghai96@gmail.com>

fix(otelcol/filter): deep copy cfgs during conversions

acb3735

Signed-off-by: hainenber <dotronghai96@gmail.com>

doc(ref/comp): remove unused MD ref

8c9eb08

Signed-off-by: hainenber <dotronghai96@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

52eef29

…r.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

9f20f73

…r.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

82e19b3

…r.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

Apply suggestions from code review

18b8d5a

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

2cb8a05

…r.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

doc(flow/ref): wordsmith warning section

3c22bba

Signed-off-by: hainenber <dotronghai96@gmail.com>

chore(dep): bump filterprocessor mod

313a278

Signed-off-by: hainenber <dotronghai96@gmail.com>

hainenber force-pushed the add-otelcol-processor-filter branch from f52fc18 to 313a278 Compare October 28, 2023 06:24

ptodev approved these changes Oct 30, 2023

View reviewed changes

hainenber and others added 5 commits October 31, 2023 00:39

Update component/otelcol/processor/filter/filter_test.go

b615fe1

Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

Update docs/sources/flow/reference/components/otelcol.processor.filte…

e4c0b37

…r.md Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

refactor(comp/otelcol): move convert() to types.go

eec95ce

Signed-off-by: hainenber <dotronghai96@gmail.com>

doc(src/flow/ref): add ref to use River raw string

623177e

Signed-off-by: hainenber <dotronghai96@gmail.com>

Merge branch 'main' into add-otelcol-processor-filter

8755163

Merge branch 'main' into add-otelcol-processor-filter

527bff5

rfratto requested a review from ptodev October 31, 2023 16:16

ptodev approved these changes Oct 31, 2023

View reviewed changes

ptodev merged commit 481674a into grafana:main Oct 31, 2023
8 checks passed

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 21, 2024

github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(comp/otelcol): component to filter OTEL telemetry #5562

feat(comp/otelcol): component to filter OTEL telemetry #5562

hainenber commented Oct 22, 2023 •

edited

Loading

ptodev left a comment

hainenber commented Oct 23, 2023

clayton-cornell left a comment

clayton-cornell Oct 24, 2023

hainenber Oct 28, 2023

hainenber Oct 30, 2023

ptodev commented Oct 26, 2023

ptodev commented Oct 26, 2023

hainenber commented Oct 28, 2023

ptodev left a comment

ptodev Oct 30, 2023

ptodev Oct 30, 2023

hainenber Oct 30, 2023

hainenber commented Oct 30, 2023

feat(comp/otelcol): component to filter OTEL telemetry #5562

feat(comp/otelcol): component to filter OTEL telemetry #5562

Conversation

hainenber commented Oct 22, 2023 • edited Loading

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

ptodev left a comment

Choose a reason for hiding this comment

hainenber commented Oct 23, 2023

clayton-cornell left a comment

Choose a reason for hiding this comment

clayton-cornell Oct 24, 2023

Choose a reason for hiding this comment

hainenber Oct 28, 2023

Choose a reason for hiding this comment

hainenber Oct 30, 2023

Choose a reason for hiding this comment

ptodev commented Oct 26, 2023

ptodev commented Oct 26, 2023

hainenber commented Oct 28, 2023

ptodev left a comment

Choose a reason for hiding this comment

ptodev Oct 30, 2023

Choose a reason for hiding this comment

ptodev Oct 30, 2023

Choose a reason for hiding this comment

hainenber Oct 30, 2023

Choose a reason for hiding this comment

hainenber commented Oct 30, 2023

hainenber commented Oct 22, 2023 •

edited

Loading