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

[processor/interval] Create the initial structure for the new processor #30756

Merged
merged 11 commits into from
Jan 26, 2024

Conversation

RichieSams
Copy link
Contributor

Description:
Adds a new processor that aggregates metrics and periodically forwards the latest values to the next component in the pipeline.

NOTE: As per the CONTRIBUTING.md recommendations, this PR only creates the basic structure of the processor. The concrete implementation will come as a separate followup PR

Link to tracking Issue:
#29461

Testing:
This is just the basic structure, so no tests yet, other than the auto-generated ones for metadata.yaml

Documentation:

This adds the basic README, but more will be added in the followup PR which adds the concrete implementation

@djaglowski
Copy link
Member

@RichieSams, looks like the collector modules need updating again. Feel free to ping me to restart CI

@RichieSams
Copy link
Contributor Author

RichieSams commented Jan 25, 2024

@djaglowski There are failing CI checks, but it looks to be all go.mod version mismatch issues. I keep rebasing to HEAD, but versions are changing faster than I can rebase and re-test. :(

How do you suggest we proceed?

Add a new processor that aggregates metrics and periodically forwards the latest values to the next component in the pipeline
And to satisfy the tests, since it's not in components.go yet
@RichieSams
Copy link
Contributor Author

@djaglowski I think we're good now to re-trigger CI. 🤞

@djaglowski
Copy link
Member

@RichieSams, apologies for the churn here. I think we got unlucky trying to do this while a release is being cut. Still, we cannot merge without passing all required checks. I've restarted CI.

@RichieSams
Copy link
Contributor Author

@djaglowski One of the checks if failing because my metadata.yml doesn't have values in the stability section:

2024/01/25 20:53:29 failed loading /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/processor/intervalprocessor/metadata.yaml: missing stability

I removed that section though, because another check wants to make sure that the component exists in components.go. But according to the CONTRIBUTING.md we don't add that until it's "ready".

Can you assist?

@djaglowski
Copy link
Member

@RichieSams, I think we need the stability section and it's ok for the component to be in components.go. I couldn't find the language you're referring to, regarding it being "ready", but I suspect it's out of date.

@github-actions github-actions bot added the cmd/otelcontribcol otelcontribcol command label Jan 25, 2024
@RichieSams
Copy link
Contributor Author

Ah, I may have been confused about some of the language here: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-new-components

I also didn't realize there is a development stability. So I used that. I think this should be good then.

@djaglowski djaglowski merged commit b1c9374 into open-telemetry:main Jan 26, 2024
85 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 26, 2024
@RichieSams
Copy link
Contributor Author

Thanks for helping get it in! I'm working on the implementation. Initial PR probably tomorrow or next week

@RichieSams RichieSams deleted the processor/interval branch January 26, 2024 13:07
@songy23
Copy link
Member

songy23 commented Jan 26, 2024

@djaglowski
Copy link
Member

Fixed by #30790. Thanks @mwear!

cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Feb 1, 2024
anthoai97 pushed a commit to anthoai97/opentelemetry-collector-contrib that referenced this pull request Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants