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

[builder] Support for --skip-new-go-module #9631

Closed

Conversation

kristinapathak
Copy link
Contributor

A continuation of #9253

Description: Adds a --skip-new-go-module flag to the OTC builder. This enables users working in an existing go module environment (say, a "monorepo") to update the module they have, vs forcing the use of a new module.

With the new support inside an existing Go module, a collector main package can be generated using a go:generate directive. For example, in the directory where I want my collector built, the file generate.go has this line:

//go:generate builder --skip-new-go-module --skip-compilation --strict-versioning --config=./build-config.yaml
In the same directory, the build-config.yaml describes the collector to build. The builder generates the other files in the same directory. At this point, normal Go workflows can be used to update indirect dependencies. The optional --strict-versioning checks that the build configuration matches that resulting from the enclosing Go module.

Link to tracking Issue: #9252

Testing: Unit tests added.

Documentation: Yes.

@kristinapathak kristinapathak requested review from a team and bogdandrutu February 23, 2024 03:26
Copy link

codecov bot commented Feb 23, 2024

Codecov Report

Attention: Patch coverage is 55.08982% with 75 lines in your changes are missing coverage. Please review.

Project coverage is 90.71%. Comparing base (b8690b6) to head (b7d0161).
Report is 76 commits behind head on main.

❗ Current head b7d0161 differs from pull request most recent head cedc4bc. Consider uploading reports for the commit cedc4bc to get more accurate results

Files Patch % Lines
cmd/builder/internal/builder/main.go 49.26% 54 Missing and 15 partials ⚠️
cmd/builder/internal/command.go 25.00% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9631      +/-   ##
==========================================
- Coverage   91.34%   90.71%   -0.63%     
==========================================
  Files         357      353       -4     
  Lines       19196    18773     -423     
==========================================
- Hits        17534    17030     -504     
- Misses       1334     1403      +69     
- Partials      328      340      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kristinapathak kristinapathak force-pushed the builder-skip-module branch 3 times, most recently from 9240936 to efce7a9 Compare March 6, 2024 03:07
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Mar 28, 2024
@dmitryax dmitryax removed the Stale label Apr 3, 2024
@mx-psi mx-psi requested a review from jpkrohling April 3, 2024 16:47
@kristinapathak
Copy link
Contributor Author

Discussed at the collector sig meeting today - I will look into splitting up the strict versioning and skip new go module flag additions into their own PRs.

@jpkrohling
Copy link
Member

I will look into splitting up the strict versioning and skip new go module flag additions into their own PRs.

Please ping me directly once you have them ready, I'll review the PRs.

Comment on lines 169 to 185
### Strict versioning checks

There is an option that controls version strictness applied by the
builder. When the `--strict-versioning` command-line flag is used,
the following additional checks apply to the finished `go.mod` file
after getting all the components and tidying the result.

1. The `dist::otelcol_version` field in the build configuration must
match the core library version calculated by the Go toolchain,
considering all components. A mismatch could happen, for example,
when one of the components depends on a newer release of the core
collector library.
2. For each component in the build configuration, the version included
in the `gomod` module specifier must match the one calculated by
the Go toolchain, considering all components. A mismatch could
happen, for example, when the enclosing Go module uses a newer
release of the core collector library.
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be the default, and would have prevented issues such as #8692. Let's do this on a separate PR, I am happy to review that one :)

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Apr 20, 2024
@kristinapathak
Copy link
Contributor Author

I'll be rebasing this and adding tests tonight.

@kristinapathak
Copy link
Contributor Author

Declaring PR bankruptcy and trying again here.

@kristinapathak kristinapathak deleted the builder-skip-module branch May 7, 2024 03:44
mx-psi added a commit that referenced this pull request Aug 22, 2024
A continuation of
#9253 and
#9631

Description: Adds a `--skip-new-go-module` flag to the OTC builder. This
enables users working in an existing go module environment (say, a
"monorepo") to update the module they have, vs forcing the use of a new
module.

With the new support inside an existing Go module, a collector main
package can be generated using a go:generate directive. For example, in
the directory where I want my collector built, the file generate.go has
this line:

//go:generate builder --skip-new-go-module --skip-compilation
--strict-versioning --config=./build-config.yaml
In the same directory, the build-config.yaml describes the collector to
build. The builder generates the other files in the same directory. At
this point, normal Go workflows can be used to update indirect
dependencies.

Link to tracking Issue:
#9252

Testing: Will add unit tests in the next few days.

Documentation: Yes.

---------

Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants