[receiver/gitproviderreceiver] rename to githubreceiver
Description: <Describe what has changed.>
Renames gitproviderreceiver to githubreceiver to better align with OpenTelemetry Semantic Conventions v1.27.0+ and allow for tracing and log signals in the future.
After the rename, the "provider" part in the name seems a bit (more) awkward. (I think it was me who originally came up with this ðĪŠ). I'm not sure what other name would be better.
- VCS receiver?
- VCS Metrics receiver?
- VCS Repository receiver?
- Repository Change Metrics receiver? ð
- ...
I had the same thought @andrzej-stencel. Technically it's still whatever a VCS provides, however, I think vcsmetricsreceiver might be a better option since it's focused on metrics only from version control systems.
@andrzej-stencel - change has been made.
For whoever merges, check-links will fail as it points to main. It will auto resolve once this PR is merged.
Out of ignorance, is there any potential of this receiver being able to support logs or traces in the future? (If so, we may not want to have
metricsin the name.)
That is a good question. I never planned for that. There is a GitHub Action Receiver that I'm getting ready to contribute that will receiver events from GitHub, turn them into traces, and additional receive the workflow logs. In that structure, it's essentially a web hook receiver.
Is there precedence where both scraping & web hook style receiving occur within the same receiver component? -- If so, I could see that tracing component being combined with this one?
I'm aware that more discussion is going on in other issues and CNCF Slack around direction to go here, but I'm approving as I'm fine either way. We can move this PR forward once there's a resolution there. (If I'm understanding properly).
sounds good, appreciate it @crobert-1 ððž
Cross posting the outcome from the SIG decision today https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/27460#issuecomment-2302537838
will update this PR to reflect.
This is now ready for merge. In future iterations I may make some small internal shifts given that we will be allowing for more signals within this singular component. This component has now been renamed to be the githubreceiver and will contain other signals.
@andrzej-stencel I know you've already approved, but would you be up for reviewing again with this new rename? Just want to make sure we're agreed here. ð
@andrzej-stencel I know you've already approved, but would you be up for reviewing again with this new rename? Just want to make sure we're agreed here. ð
Sure! :rocket:
Thanks for making the rename! One potential change here would be to move the
internal/scraper/githubscraper/directory up a level to just beinternal/scraper, since there's no need to qualify which scraper is being used. Just a thought though, not needed as a part of the rename.
I agree! that's the next thing to do outside this PR!
I think we can merge this once conflicts are resolved
@mx-psi - conflicts resolved. Unrelated and unexpected failure observed in build & test workflows for the redaction processor. Fixed once #34937 is merged. All workflows are good otherwise.
Looks like this
[â] https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/githubreceiver/internal/scraper/githubscraper/README.md#github-limitations â Status: 404
Is a real error though?
Looks like this
[â] https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/githubreceiver/internal/scraper/githubscraper/README.md#github-limitations â Status: 404Is a real error though?
@mx-psi it will be resolved once merged into main because this is going to a main ref for the documentation which doesn't currently exist given that this is a component rename