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

[choe] replace scraperhelper.NewScraperWithComponentType to scraperhelper.NewScraper as deprecated #35205

Closed
wants to merge 4 commits into from

Conversation

mrasu
Copy link

@mrasu mrasu commented Sep 16, 2024

To change an argument of scraperhelper.NewScraper, scraperhelper.NewScraperWithComponentType is introduced in previous version and deprecated in newer version at open-telemetry/opentelemetry-collector#11159.

This PR update dependencies by multimod sync and replace scraperhelper.NewScraperWithComponentType to scraperhelper.NewScraper

Link to tracking Issue: open-telemetry/opentelemetry-collector#9473

@@ -4,7 +4,7 @@ module github.com/open-telemetry/opentelemetry-collector-contrib/cmd/otelcontrib

go 1.22.0

toolchain go1.22.7
toolchain go1.23.1
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't update the toolchain version here. (Same comment for other toolchain updates)

gopkg.in/yaml.v3 v3.0.1
)

require github.com/rogpeppe/go-internal v1.12.0 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

Can you share why this dependency is required? I wouldn't have expected a new dependency to be required when the only change is updating existing dependencies, but one of them must require it now?

@mrasu
Copy link
Author

mrasu commented Sep 16, 2024

@crobert-1
Sorry. I ran make update-otel but encountered errors saying go.opentelemetry.io/collector/internal/localhostgate was not found during gotidy.
So, I fixed this by running go mod edit -require go.opentelemetry.io/collector/config/[email protected] and then executed the commands after gotidy in update-otel manually.
However, it made huge diffs including unrelated changes as you mentioned.

So, I recreated the changes by running multimod sync -s=true -o ../opentelemetry-collector -m beta,stable --commit-hash 6029f31767 and fixed the errors manually.

@mx-psi
Copy link
Member

mx-psi commented Sep 17, 2024

Overlaps with #35215, I think @codeboten wants to merge #35215 first

@codeboten
Copy link
Contributor

sorry @mrasu! i hadnt seen this PR before i started working on my own update core PR to include the changes i made to the processor helper... i think it's almost ready to merge

@mrasu
Copy link
Author

mrasu commented Sep 18, 2024

@codeboten No worries! close this

@mrasu mrasu closed this Sep 18, 2024
@mrasu mrasu deleted the deprecated-new-scraper branch September 18, 2024 13:09
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.

5 participants