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

Kbauer/use dockerized ocb #161

Closed
wants to merge 15 commits into from
Closed

Conversation

kb-newrelic
Copy link
Contributor

@kb-newrelic kb-newrelic commented Dec 3, 2024

Summary

  • Use dockerized ocb instead of manually compiling ocb
  • Versioning
    • ocb version is implicitly defined by otelcol_version in manifest. This also gets rid of version in build.mk potentially diverging from version in manifest
    • oldest version of dockerized ocb is 0.112.0. Compiling older versions of component with it, caused all kind of weird errors, so I ended up having to bump versions to 0.112.0
      • >= 0.113.0 removes support for otelcol_version in manifest, so I decided to leave that version bump for a separate PR to not complicate things even more.
      • The only relevant change for us is loggingexporter getting replaced by debugexporter, see also this draft PR we already had pending
  • Refactored makefiles to
    • simplify debugging
    • avoid unnecessary builds by defining file dependencies
    • pave the way for maintaining multiple distros by using delegation targets for build (previously only for checks)

Makefile refactoring

As part of debugging the license file generatio, I ended up refactoring and hopefully simplifying the Makefile structure as the old structure was copied from the otel repo and is more complicated than we need it to be.

  • moved 'delegation targets' generated for each distro from check.mk to Makefile as they were using the variable ALL_DISTRIBUTIONS which was defined there and are now also used in the build targets, so it makes sense to have defined in the 'root' Makefile.
  • 'build' targets were modified to be distro-specific and to not run if sources (_build/go.mod acting as representative) or binary are already created. They were also moved from build.mk to Makefile.common where the already distro-specific 'check' targets where defined. build.mk and check.mk ar included in Makefile and include top-level targets to build/check all distros.
  • changed 'license' targets to depend on 'generate-sources' instead of 'build' as they only require the sources, not the binary.

Misc

  • go version is implictly defined by the ocb image, so no need to allow specifying it
  • Explicitly define providers instead of relying on ocb defaults, see also 0.111.0 known bugs

@kb-newrelic kb-newrelic requested a review from a team as a code owner December 3, 2024 00:40
@kb-newrelic kb-newrelic force-pushed the kbauer/use-dockerized-ocb branch from ae68516 to 038b92d Compare December 3, 2024 00:47
@@ -21,166 +21,6 @@ t/.



## [github.com/open-telemetry/opentelemetry-collector-contrib/exporter/prometheusremotewriteexporter](https://github.com/open-telemetry/opentelemetry-collector-contrib)
Copy link
Contributor Author

@kb-newrelic kb-newrelic Dec 5, 2024

Choose a reason for hiding this comment

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

TODO: figure out where discrepancy between local machine and CI comes from (build is failing)

Copy link
Contributor Author

@kb-newrelic kb-newrelic Dec 10, 2024

Choose a reason for hiding this comment

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

  • diff is due to ci generating an empty license file, so the new build process is not yet compatible with ci
  • still unclear why my local machine generates a different license file than mailo-nr on their machine for a very similar change

@kb-newrelic kb-newrelic force-pushed the kbauer/use-dockerized-ocb branch from e9d8a2b to c4eeb95 Compare December 9, 2024 18:59
@kb-newrelic kb-newrelic force-pushed the kbauer/use-dockerized-ocb branch from 72a5bbd to 3b650b9 Compare December 10, 2024 19:32
@kb-newrelic kb-newrelic force-pushed the kbauer/use-dockerized-ocb branch from 7081b81 to 8598a7f Compare December 10, 2024 19:41
@kb-newrelic
Copy link
Contributor Author

Closing this to reopen a new PR as I can't wrap my head around the license file discrepancies and want to rule out build machine reuse.

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.

1 participant