Skip to content

Conversation

@tomatopunk
Copy link

Description

This PR adds a new package option to the builder configuration, allowing the generated OpenTelemetry Collector sources to use a custom package name instead of main. When the package is set to a value other than main, the build process is updated to recognize that the output is intended as a Go library for import by other Go modules, rather than an executable binary.

Logging is improved to provide clear messaging about this build mode.

Link to tracking issue

Fixes #

Testing

Documentation

Added documentation for the new package option in the example builder configuration and usage docs.
Clarified build output expectations when using non-main packages.

@tomatopunk tomatopunk requested a review from a team as a code owner June 30, 2025 07:02
@tomatopunk tomatopunk requested a review from atoulme June 30, 2025 07:02
@dmathieu
Copy link
Member

This should be tested.

@tomatopunk
Copy link
Author

This should be tested.

Added two improvements in the unit tests:

  1. enhanced config_test to validate the default config
  2. after running Generate, added a check to ensure the package statement in each generated Go file matches Distribution.Package

# Conflicts:
#	cmd/builder/internal/config/default.yaml
@codecov
Copy link

codecov bot commented Jul 1, 2025

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.58%. Comparing base (7e45e34) to head (2279efe).
⚠️ Report is 112 commits behind head on main.

Files with missing lines Patch % Lines
cmd/builder/internal/builder/main.go 70.00% 2 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (72.72%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13296   +/-   ##
=======================================
  Coverage   91.57%   91.58%           
=======================================
  Files         529      530    +1     
  Lines       31460    31474   +14     
=======================================
+ Hits        28811    28826   +15     
+ Misses       2083     2080    -3     
- Partials      566      568    +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tomatopunk tomatopunk marked this pull request as draft July 9, 2025 10:38
@tomatopunk tomatopunk marked this pull request as ready for review July 9, 2025 14:09
@tomatopunk
Copy link
Author

Hi @dmathieu
I noticed that on Windows, the builder was not adding the .exe extension to executable files. I’ve submitted a fix for this issue. Could you please trigger the CI? Thank you!

@github-actions
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 Jul 25, 2025
@tomatopunk
Copy link
Author

Hello @dmathieu,

I've encountered two issues:

I fixed the problem where compiled Windows files were missing the .exe extension. However, since Codecov runs on Ubuntu, these lines are actually being tested, but Codecov still shows them as uncovered. Is it possible to skip this issue and force the merge?
While running CI, I noticed some problems with contrib-test-matrix, specifically with exporter-0, exporter-1, and internal jobs. Based on the error messages, it seems like there may be version incompatibilities. How should I go about resolving these issues?

@dmathieu
Copy link
Member

Codecov issues are not a blocker. You can ignore them.

@tomatopunk
Copy link
Author

Hello @dmathieu,
I have fixed the issue in the unit tests where compilation was failing due to different package versions. Could you please trigger the CI again? Thank you!

@tomatopunk
Copy link
Author

Hello @dmathieu,

There are two CI errors remaining: builder-snapshot and spell-check. It seems that spell-check is checking all the YAML files. Do I need to fix both of these errors before I can merge the code?

@github-actions github-actions bot removed the Stale label Jul 26, 2025
@dmathieu
Copy link
Member

The spellcheck is fine. We currently have issues with it and golangci.yml.
The snapshot build is more problematic though, as it indicates a regression in your PR, that should be fixed.

@tomatopunk
Copy link
Author

The spellcheck is fine. We currently have issues with it and golangci.yml. The snapshot build is more problematic though, as it indicates a regression in your PR, that should be fixed.

Hello @dmathieu,

I've noticed that the snapshot build is consistently failing, as seen in #13493 and #13494.

It appears the issue is related to the "Copy release files" step—specifically, the .git directory is not being copied.

For this PR, I suggest we ignore the snapshot error. I will create a new PR to fix the snapshot build issue. However, in order to debug this further, I am unable to manually trigger the CI. Could someone with the required permissions please help re-run the workflow for me?

Thank you!

@tomatopunk
Copy link
Author

The spellcheck is fine. We currently have issues with it and golangci.yml. The snapshot build is more problematic though, as it indicates a regression in your PR, that should be fixed.

Hello @dmathieu,

I've noticed that the snapshot build is consistently failing, as seen in #13493 and #13494.

It appears the issue is related to the "Copy release files" step—specifically, the .git directory is not being copied.

For this PR, I suggest we ignore the snapshot error. I will create a new PR to fix the snapshot build issue. However, in order to debug this further, I am unable to manually trigger the CI. Could someone with the required permissions please help re-run the workflow for me?

Thank you!

I’ve already created a PR to fix the issue of the snapshot builder being blocked. Hopefully, it will be merged soon!
#1051

# Conflicts:
#	cmd/builder/internal/config/default.yaml
@tomatopunk
Copy link
Author

hello @dmathieu
the PR to fix the snapshot builder has been merged, and I have resolved the conflicts. Could you please help trigger the CI? Thank you!

@tomatopunk
Copy link
Author

hello @dmathieu,
all check have passed, can we merge this pr?

@dmathieu
Copy link
Member

This PR hasn't been approved yet, so no it can't be merged.
It's also a non-trivial change, so I think we should get multiple eyes on it.

@github-actions
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 Aug 14, 2025
@tomatopunk
Copy link
Author

This pull request has become stale, and there is no clear process in place to indicate it will be approved, so I have decided to close it.

@tomatopunk tomatopunk closed this Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants