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

Removing go mod replace for collector/service and collector/otelcol #1023

Closed
wants to merge 1 commit into from

Conversation

ptodev
Copy link
Contributor

@ptodev ptodev commented Jun 11, 2024

In the spirit of "a little copying is better than a little dependency."....

I will also make a similar change to Agent. However, in the Agent I think we will still need the "service" package to be replaced so that we can embed the OTel Collector's metrics in the Agent's /metrics endpoint.

@ptodev
Copy link
Contributor Author

ptodev commented Jun 11, 2024

This copying is not "a little", but I think it's still better than the go mod replace 😄

Copy link
Contributor

This PR has not had any activity in the past 30 days, so the needs-attention label has been added to it.
If you do not have enough time to follow up on this PR or you think it's no longer relevant, consider closing it.
The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your PR will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity.
Thank you for your contributions!

@ptodev ptodev force-pushed the ptodev/rid-of-otel-fork branch from 6689dfb to b51d740 Compare November 20, 2024 17:52
@ptodev ptodev requested a review from a team as a code owner November 20, 2024 17:52
// https://github.com/open-telemetry/opentelemetry-collector/issues/4970
replace (
go.opentelemetry.io/collector/otelcol => github.com/grafana/opentelemetry-collector/otelcol v0.0.0-20241104164848-8ea9d0a3e17a
go.opentelemetry.io/collector/processor/batchprocessor => github.com/grafana/opentelemetry-collector/processor/batchprocessor v0.0.0-20241104164848-8ea9d0a3e17a
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the batch processor replace too, because this change has been merged: #2027

Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Fantastic LGTM, I didnt look to closely at the forked code assumed it was the same.

@dehaansa
Copy link
Contributor

To avoid copied code, I reimplemented the feature using the exported tools otelcol expects you to use in this other PR: #2148

@ptodev
Copy link
Contributor Author

ptodev commented Nov 28, 2024

Closing this PR in favour of #2148, which doesn't require copying code 🥳

@ptodev ptodev closed this Nov 28, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants