Skip to content

[confmap] - add a more targeted version of append-merging strategy #12926

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

Merged
merged 11 commits into from
May 5, 2025

Conversation

VihasMakwana
Copy link
Contributor

@VihasMakwana VihasMakwana commented Apr 25, 2025

Follow-up of #12097

Description

Ideally, users want to merge lists in pipelines only. In this PR, I'm adding support for merging following components:

  1. extensions
  2. receivers
  3. exporters

We make use of glob-like pattern to match the path because there can be multiple pipelines configured. Inspired by @evan-bradley's idea.

I'm leaving processors out of this list because we make an ordering guarantee.

Future plan:

Once this PR gets merged, it would make it easier for us to decide upon the command line flags to make this option configurable over the RFC. Right now, patterns is hardcoded to include above mentioned components, but we can easily make it configurable after making some tweaks.

Link to tracking issue

Relates #8754

Testing

Expanded the existing test cases

Documentation

@VihasMakwana VihasMakwana requested review from mx-psi, evan-bradley and a team as code owners April 25, 2025 20:37
Copy link

codecov bot commented Apr 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.70%. Comparing base (73e7c97) to head (14f7006).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #12926   +/-   ##
=======================================
  Coverage   91.69%   91.70%           
=======================================
  Files         503      503           
  Lines       27555    27568   +13     
=======================================
+ Hits        25267    25280   +13     
  Misses       1808     1808           
  Partials      480      480           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dmitryax
Copy link
Member

Please add a changelog entry

@VihasMakwana
Copy link
Contributor Author

Please add a changelog entry

Done.

@VihasMakwana VihasMakwana force-pushed the merge-append-components branch from 6dc7d55 to 47dd618 Compare April 26, 2025 04:42
@VihasMakwana
Copy link
Contributor Author

@mx-psi please take a look when you can. Once we get this merged, I'll open an RFC for configuring the option.

Copy link
Member

@mx-psi mx-psi 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 to me, I wonder if we have been able to get any feedback on this and understand what people prefer. I saw #12932, but I guess this is only under the print-initial-config subcommand and does not affect the usual running of the Collector

@VihasMakwana
Copy link
Contributor Author

VihasMakwana commented Apr 30, 2025

This looks good to me, I wonder if we have been able to get any feedback on this and understand what people prefer. I saw #12932, but I guess this is only under the print-initial-config subcommand and does not affect the usual running of the Collector

I've raised #12952 to fix it. Sorry, I wasn't aware about it.
Let's merge the bug fix PR first.

@VihasMakwana
Copy link
Contributor Author

VihasMakwana commented Apr 30, 2025

I wonder if we have been able to get any feedback on this and understand what people prefer

People generally prefer this option to work sensibly by default, but still be customizable using command-line flags. We can go over the details about command line flags in an RFC.

@mx-psi
Copy link
Member

mx-psi commented May 5, 2025

Happy to merge this once tests pass

@VihasMakwana
Copy link
Contributor Author

@mx-psi looks good now.

@mx-psi mx-psi added this pull request to the merge queue May 5, 2025
Merged via the queue into open-telemetry:main with commit 84888ad May 5, 2025
56 checks passed
TimoBehrendt pushed a commit to TimoBehrendt/tracebasedlogsampler that referenced this pull request May 20, 2025
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [go.opentelemetry.io/collector/component](https://github.com/open-telemetry/opentelemetry-collector) | require | minor | `v1.31.0` -> `v1.32.0` |
| [go.opentelemetry.io/collector/component/componenttest](https://github.com/open-telemetry/opentelemetry-collector) | require | minor | `v0.125.0` -> `v0.126.0` |
| [go.opentelemetry.io/collector/confmap](https://github.com/open-telemetry/opentelemetry-collector) | require | minor | `v1.31.0` -> `v1.32.0` |
| [go.opentelemetry.io/collector/consumer](https://github.com/open-telemetry/opentelemetry-collector) | require | minor | `v1.31.0` -> `v1.32.0` |
| [go.opentelemetry.io/collector/consumer/consumertest](https://github.com/open-telemetry/opentelemetry-collector) | require | minor | `v0.125.0` -> `v0.126.0` |
| [go.opentelemetry.io/collector/pdata](https://github.com/open-telemetry/opentelemetry-collector) | require | minor | `v1.31.0` -> `v1.32.0` |
| [go.opentelemetry.io/collector/processor](https://github.com/open-telemetry/opentelemetry-collector) | require | minor | `v1.31.0` -> `v1.32.0` |
| [go.opentelemetry.io/collector/processor/processortest](https://github.com/open-telemetry/opentelemetry-collector) | require | minor | `v0.125.0` -> `v0.126.0` |

---

### Release Notes

<details>
<summary>open-telemetry/opentelemetry-collector (go.opentelemetry.io/collector/component)</summary>

### [`v1.32.0`](https://github.com/open-telemetry/opentelemetry-collector/blob/HEAD/CHANGELOG.md#v1320v01260)

##### 🛑 Breaking changes 🛑

-   `configauth`: Removes deprecated `configauth.Authentication` and `extensionauthtest.NewErrorClient` ([#&#8203;12992](open-telemetry/opentelemetry-collector#12992))
    The following have been removed:
    -   `configauth.Authentication` use `configauth.Config` instead
    -   `extensionauthtest.NewErrorClient` use `extensionauthtest.NewErr` instead

##### 💡 Enhancements 💡

-   `service`: Replace `go.opentelemetry.io/collector/semconv` usage with `go.opentelemetry.io/otel/semconv` ([#&#8203;12991](open-telemetry/opentelemetry-collector#12991))
-   `confmap`: Update the behavior of the confmap.enableMergeAppendOption feature gate to merge only component lists. ([#&#8203;12926](open-telemetry/opentelemetry-collector#12926))
-   `service`: Add item count metrics defined in Pipeline Component Telemetry RFC ([#&#8203;12812](open-telemetry/opentelemetry-collector#12812))
    See [Pipeline Component Telemetry RFC](https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/rfcs/component-universal-telemetry.md) for more details:
    -   `otelcol.receiver.produced.items`
    -   `otelcol.processor.consumed.items`
    -   `otelcol.processor.produced.items`
    -   `otelcol.connector.consumed.items`
    -   `otelcol.connector.produced.items`
    -   `otelcol.exporter.consumed.items`
-   `tls`: Add trusted platform module (TPM) support to TLS authentication. ([#&#8203;12801](open-telemetry/opentelemetry-collector#12801))
    Now the TLS allows the use of TPM for loading private keys (e.g. in TSS2 format).

##### 🧰 Bug fixes 🧰

-   `exporterhelper`: Add validation error for batch config if min_size is greater than queue_size. ([#&#8203;12948](open-telemetry/opentelemetry-collector#12948))

-   `telemetry`: Allocate less memory per component when OTLP exporting of logs is disabled ([#&#8203;13014](open-telemetry/opentelemetry-collector#13014))

-   `confmap`: Use reflect.DeepEqual to avoid panic when confmap.enableMergeAppendOption feature gate is enabled. ([#&#8203;12932](open-telemetry/opentelemetry-collector#12932))

-   `internal telemetry`: Add resource attributes from telemetry.resource to the logger ([#&#8203;12582](open-telemetry/opentelemetry-collector#12582))
    Resource attributes from telemetry.resource were not added to the internal
    console logs.

    Now, they are added to the logger as part of the "resource" field.

-   `confighttp and configcompression`: Fix handling of `snappy` content-encoding in a backwards-compatible way ([#&#8203;10584](open-telemetry/opentelemetry-collector#10584), [#&#8203;12825](open-telemetry/opentelemetry-collector#12825))
    The collector used the Snappy compression type of "framed" to handle the HTTP
    content-encoding "snappy".  However, this encoding is typically used to indicate
    the "block" compression variant of "snappy".  This change allows the collector to:
    -   When receiving a request with encoding 'snappy', the server endpoints will peek
        at the first bytes of the payload to determine if it is "framed" or "block" snappy,
        and will decompress accordingly.  This is a backwards-compatible change.
    If the feature-gate "confighttp.framedSnappy" is enabled, you'll see new behavior for both client and server:
    -   Client compression type "snappy" will now compress to the "block" variant of snappy
        instead of "framed". Client compression type "x-snappy-framed" will now compress to the "framed" variant of snappy.
    -   Servers will accept both "snappy" and "x-snappy-framed" as valid content-encodings.

-   `tlsconfig`: Disable TPM tests on MacOS/Darwin ([#&#8203;12964](open-telemetry/opentelemetry-collector#12964))

<!-- previous-version -->

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4yNjMuMSIsInVwZGF0ZWRJblZlciI6IjM5LjI2My4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Reviewed-on: https://gitea.t000-n.de/t.behrendt/tracebasedlogsampler/pulls/13
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants