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.

Sign up for GitHub

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

Jump to bottom

docs: Custom OTTL with OpenTelemetry-based Telemetry Pipelines #1520

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

docs: Custom OTTL with OpenTelemetry-based Telemetry Pipelines #1520

wants to merge 35 commits into from

Conversation

hisarbalik
Copy link
Contributor

Description

Changes proposed in this pull request (what was done and why):

  • ADR, custom OTTL with OpenTelemetry-based Pipelines

Changes refer to particular issues, PRs or documents:

Traceability

  • The PR is linked to a GitHub issue.
  • The follow-up issues (if any) are linked in the Related Issues section.
  • If the change is user-facing, the documentation has been adjusted.
  • If a CRD is changed, the corresponding Busola ConfigMap has been adjusted.
  • The feature is unit-tested.
  • The feature is e2e-tested.

@hisarbalik hisarbalik requested a review from a team as a code owner October 14, 2024 04:40
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 14, 2024
@hisarbalik hisarbalik added area/documentation Documentation changes and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cla: yes Indicates the PR's author has signed the CLA. labels Oct 14, 2024
@hisarbalik hisarbalik added this to the 1.26.0 milestone Oct 14, 2024
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 14, 2024
@hisarbalik hisarbalik added area/metrics MetricPipeline area/traces TracePipeline kind/docs Categorizes issue or PR as related to a documentation change labels Oct 14, 2024
NHingerl
NHingerl reviewed Oct 14, 2024
View reviewed changes
docs/contributor/arch/015-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/015-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/015-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/015-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/015-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/015-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/015-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/015-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
@a-thaler a-thaler modified the milestones: 1.26.0, 1.27.0 Oct 21, 2024
@kyma-bot kyma-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 29, 2024
@hisarbalik hisarbalik requested a review from k15r November 4, 2024 07:07
@a-thaler a-thaler modified the milestones: 1.27.0, 1.28.0 Nov 5, 2024
@hisarbalik hisarbalik requested a review from k15r November 6, 2024 07:12
@hisarbalik hisarbalik requested a review from k15r November 6, 2024 08:46
NHingerl
NHingerl reviewed Nov 6, 2024
View reviewed changes
docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md Outdated Show resolved Hide resolved
docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md Outdated

To mitigate this risk, we suggest the following measures:
- The [beta version](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/28892) of the OTTL library will be used in the OTel Collector to ensure that the library is stable and reliable.
- New Versions with Breaking Changes: New versions with [breaking changes](https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/coding-guidelines.md#breaking-changes) of the OTTL library will be rolled out with backward compatibility. This will help users adapt changes and reduce the chance of widespread disruption.
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a measure we can take? Do we make sure that new versions are rolled out with backward compatibility? The passive phrase "will be rolled out" doesn't indicate who takes care of that; or what our contribution to this mitigative action 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.

The point reworked

docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md Outdated
- Thorough Testing: We will maintain strict version control of the OTTL library, ensuring that each update is thoroughly tested. Automated tests will validate that existing pipelines continue to work as expected.
- Documentation: We will provide detailed documentation and practical examples of how to configure filter and transformation processors using OTTL expressions.
- Community Support: We will actively engage with the OpenTelemetry community to address any issues or concerns that arise from using the OTTL library.
- The public Go API: The breaking changes most likely will not follow deprecation policy, this may require changes in our implementation.
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
- The public Go API: The breaking changes most likely will not follow deprecation policy, this may require changes in our implementation.
- The public Go API: The breaking changes most likely will not follow deprecation policy. This may require changes in our implementation.

Not sure how this is a mitigation measure; it's just a statement. Why will the breaking changes (the ones from line 114, I assume?) not follow (which?) deprecation strategy? Which specific mitigative action does this require? Something like "We must design our implementation to cope with that." or "We must continuously monitor this and change our implementation accordingly"? or something similar?

Copy link
Contributor Author

@hisarbalik hisarbalik Nov 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I removed this point from the mitigation, this is more an exception to mention

@hisarbalik hisarbalik removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 7, 2024
NHingerl
NHingerl reviewed Nov 12, 2024
View reviewed changes
docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md

To mitigate this risk, we suggest the following measures:
- The [beta version](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/28892) of the OTTL library will be used in the OTel Collector to ensure that the library is stable and reliable.
- New Versions with Breaking Changes: The [breaking changes](https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/coding-guidelines.md#breaking-changes) of the OTTL library will be released with backward compatibility, the backward compatibility provided via `feature gates` and will be available over several releases. This will help users adapt changes and reduce the chance of widespread disruption.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a measure we can take? Do we make sure that new versions are released with backward compatibility? The passive phrase "will be released" doesn't indicate who takes care of that; or what our contribution to this mitigative action is. Or, coming to the intro sentence, do we suggest this to the OTel team? Are we asking them to make sure their releases are backward compatible?

I see you changed this paragraph but it's still all passive voice, and it's not clear to me who's taking care of this mitigation measure.

docs/contributor/arch/016-transform-filter-support-for-telemetry-pipelines.md
- Community Support: We will actively engage with the OpenTelemetry community to address any issues or concerns that arise from using the OTTL library.


> **NOTE:** The public Go API (not the user faced features), the breaking changes most likely will not follow deprecation policy, this may require changes in our implementation uses the public API.
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you formatted this now as a note, not a bullet point. Still, it's not clear to me how this affects our implementation, my previous questions still stand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NHingerl NHingerl NHingerl left review comments

@k15r k15r Awaiting requested review from k15r

At least 1 approving review is required to merge this pull request.

Assignees

@k15r k15r

@NHingerl NHingerl

Labels
area/documentation Documentation changes area/metrics MetricPipeline area/traces TracePipeline cla: yes Indicates the PR's author has signed the CLA. kind/docs Categorizes issue or PR as related to a documentation change size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
1.28.0
Development

Successfully merging this pull request may close these issues.
6 participants
@hisarbalik @k15r @NHingerl @a-thaler @kyma-bot @skhalash