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

Add otel-plugin-nginx, otel-cpp #30305

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

wlynch
Copy link
Contributor

@wlynch wlynch commented Oct 8, 2024

  • Adds opentelemetry-cpp: c++ client for otel (dependency for otel-plugin-nginx).
  • Adds otel-plugin-nginx
  • Modifies protobuf to disable generation of libupb. This is a static internal dependency of protobuf. If this ends up being load bearing for php and python protobuf packages, we should repackage this into a subpackage.

Related: https://github.com/chainguard-dev/image-requests/issues/4407

Pre-review Checklist

For new package PRs only

  • This PR is marked as fixing a pre-existing package request bug
    • Alternatively, the PR is marked as related to a pre-existing package request bug, such as a dependency
  • REQUIRED - The package is available under an OSI-approved or FSF-approved license
  • REQUIRED - The version of the package is still receiving security updates
  • This PR links to the upstream project's support policy (e.g. endoflife.date)

@wlynch wlynch force-pushed the otel-nginx branch 5 times, most recently from b525449 to f4b90f4 Compare October 8, 2024 16:38
wlynch added a commit that referenced this pull request Oct 9, 2024
Removes upb which is causing issues with downstream dependencies
(otel-nginx - see #30305). This is
documented as being an [internal dependency of
grpc](https://github.com/protocolbuffers/protobuf/blob/6690ab42d855ea19d9a24cd99b0375910ea772ca/cmake/libupb.cmake#L14-L15).
I did a spot check locally and py3-protobuf and php-protobuf can still
build.

Related: chainguard-dev/image-requests#4407,
#30305

There is a newer version of protobuf available, but this breaks other
packages and requires additional changes.
@wlynch wlynch force-pushed the otel-nginx branch 3 times, most recently from 75f661e to a88f7ab Compare October 9, 2024 18:09
- Adds opentelemetry-cpp: c++ client for otel (dependency for
  otel-plugin-nginx).
- Adds otel-plugin-nginx
cmake --install . --prefix ${{targets.contextdir}}/usr

- uses: strip

Copy link
Member

Choose a reason for hiding this comment

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

Any docs/infodirs we can split into subpackages, development files. etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was wondering about this - I didn't see any documentation about this in the wiki. What are the common subpackage breakdowns we aim for, what kinds of files go in them, and what naming conventions should each subpackage have?

Here's the file extension breakdown for files in the package:

   4 cmake
   7 pc
  18 so
 334 h

I can probably break out the headers, but what about cmake/pc? Are those considered development files, or should they go in a separate package?

Copy link
Member

Choose a reason for hiding this comment

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

We have pipelines in melange that can be used to split things away: https://github.com/chainguard-dev/melange/tree/main/pkg/build/pipelines/split

Typically, we split whatever we can. I.E., those headers (the .h files) would be split away with split/dev and docs would be split with split/manpages and split/info if an info directory exists. It looks like in your case, you just need to create a -dev subpackage that uses the split/dev pipeline

opentelemetry-plugin-nginx.yaml Show resolved Hide resolved
opentelemetry-cpp.yaml Outdated Show resolved Hide resolved
opentelemetry-plugin-nginx.yaml Outdated Show resolved Hide resolved
opentelemetry-plugin-nginx.yaml Outdated Show resolved Hide resolved
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.

2 participants