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

Move log-collection library into collector-contrib codebase #9227

Closed
wants to merge 2 commits into from

Conversation

djaglowski
Copy link
Member

@djaglowski djaglowski commented Apr 12, 2022

This is one of the major steps defined in #8850.

This PR copies all packages from opentelemetry-log-collection
into a new pkg/stanza module and updates paths such that
the code will compile and tests will pass. This codebase represents
the exact contents of the log-collection packages at v0.29.0,
with the following exceptions:

  • Updated module and package paths
  • Added a Makefile and doc.go
  • Updated versions.yaml
  • Omitted internal/tools module
  • Removed a couple extraneous docs
  • Removed mockery, as it was barely used
  • Resolves several minor gosec issues
  • Resolved several minor lint issues, eg:
    • misspellings in docs
    • shadow declarations of err
    • improperly cased initialisms
  • Destuttered a massive number of struct names (also for lint)
    • eg file.Input instead of file.FileInput

It does NOT change usages of the log-collection library by components
in the collector. This will follow in a subsequent PR.

@djaglowski djaglowski added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 12, 2022
@djaglowski djaglowski force-pushed the mv-log-collection branch 3 times, most recently from 0ffdf0c to 00f5a4f Compare April 12, 2022 21:15
This PR copies all packages from opentelemetry-log-collection
into a new pkg/stanza directory, and renames packages such that
the code will compile and tests will pass. This codebase represents
the exact contents of the log-collection packages as of v0.29.0,
with the exception of the updated module and package paths.

It does NOT change usages of the log-collection library by components
in the collector. This will follow in a subsequent PR.
@djaglowski
Copy link
Member Author

djaglowski commented Apr 14, 2022

This PR is massive. It would arguably be an ok approach if it were a straight copy/paste. However, it evolved into a much more diverse set of changes due to differences in lint/gosec rules. I believe a more traceable approach is needed.

Options would seem to be roughly: 1) break this PR up into smaller PRs applying lint as needed but in smaller batches, 2) copy/paste into contrib, but exclude from CI until lint updates are made, or 3) update lint standards on log-collection and apply changes in place first, then bring over, or . In all cases, changes need to be made in logical steps so that they can be reasonably followed.

Option 1 would still obfuscate changes, but it is at least straightforward and would avoid the massive lint update that would be necessary to make the other two options pass CI. Unless there are objections, I'll establish the module, bring over docs, then small core packages like entry, operator, and pipeline, then finally batch up the operators as seems appropriate.

@djaglowski
Copy link
Member Author

Closing in favor of incremental approach. #9328 is the first incremental PR.

@djaglowski djaglowski closed this Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant