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

[ottl] Demonstrate simpler context constructors #36188

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

djaglowski
Copy link
Member

This is just a proof of concept PR, rather than an issue.

I've been working with OTTL contexts lately and noticed that some of the parameters seem to be redundant. Looking back at the commit history, I believe the context constructors were created prior to some refactoring in pdata packages, so the redundancy appears to have been inherited from upstream changes.

My question is whether or not we could introduce a simpler set of constructors at this point, and potentially deprecate the old ones, maybe even push these changes down into the functions (which could access the same information).

Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

I'm not opposed to refactoring these as they have been a source of pain as OTTL has grown. We've had to change them in the past to support new parameters.

I'd much rather introduce something that scales like Options or a struct or something that can easily take a new parameter without breaking the function signature. We keep saying "this is the last parameter we'll need bc that's all the proto defines" but then there are exceptions (like schema).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants