Skip to content

feat: Add Dockerfile for OTEL enabled container images #318

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

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

Conversation

hangy
Copy link

@hangy hangy commented Jun 12, 2025

Proposed changes

Add support for building unprivileged images with OTEL support.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

  • I have read the contributing guidelines
  • I have signed the F5 Contributor License Agreement (CLA)
  • I have run the update.sh script and ensured all entrypoint/Dockerfile template changes have been applied to the relevant image entrypoint scripts & Dockerfiles
  • I have tested that the NGINX Docker unprivileged image builds and runs correctly on all supported architectures on an unprivileged environment (check out the README for more details)
  • I have updated any relevant documentation (README.md)

Additional Notes

  • This is a direct port of the -otel images from docker-nginx (lots of copy & paste) without much original input from my side.
  • I tested all 4 images against an OTEL collector and got all traces to be logged.
  • The CI pipelines are getting kinda long and hard to maintain. It might be a good idea to refactor the GitHub Actions like in docker-nginx at some point in time.

Would close #263

@Copilot Copilot AI review requested due to automatic review settings June 12, 2025 22:21
@hangy hangy requested a review from a team as a code owner June 12, 2025 22:21
Copilot

This comment was marked as outdated.

@alessfg alessfg added the enhancement New feature or request label Jun 13, 2025
Copy link
Member

@alessfg alessfg left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Copy/pasting is very much the name of the game here since the images are fundamentally meant to be identical to the upstream Docker images. I've made a couple comments but it seems almost ready to go!

@hangy hangy requested review from Copilot and alessfg June 13, 2025 17:33
Copilot

This comment was marked as outdated.

Copy link
Member

@alessfg alessfg left a comment

Choose a reason for hiding this comment

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

In addition to the other comment I left, can you also update https://github.com/nginx/docker-nginx-unprivileged/pull/318/files#diff-6456e6e208552591349a09da1db573bb5de5fdfe76776686baf7558f547f4287L31 to grep the eight iteration of mainline (grep -m7 to grep -m8), and similarly, change the Debian workflow to grep the seventh iteration of mainline? The GH workflow won't work correctly otherwise.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for building unprivileged NGINX container images with OpenTelemetry (OTEL) modules, including template and CI pipeline updates.

  • Extend update.sh to generate OTEL-enabled Dockerfiles and inject OTEL versions into package lists
  • Introduce new OTEL-specific Dockerfile templates and generated Dockerfiles for both Debian and Alpine images
  • Update GitHub Actions workflows to build and publish OTEL variants alongside existing images

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
update.sh Added otel version map, extended get_packages, get_packagerepo, and get_buildtarget for OTEL
Dockerfile-debian.template
Dockerfile-debian-perl.template
Fixed spelling and updated template comments for OTEL support
Dockerfile-debian-otel.template
Dockerfile-alpine-otel.template
New OTEL templates for Debian and Alpine
stable/debian-otel/Dockerfile
stable/alpine-otel/Dockerfile
Generated OTEL-enabled Dockerfiles for stable branch
mainline/debian-otel/Dockerfile
mainline/alpine-otel/Dockerfile
Generated OTEL-enabled Dockerfiles for mainline branch
.github/workflows/debian-stable.yml Added OTEL build job and adjusted version parsing for stable Debian
.github/workflows/debian-mainline.yml Added OTEL build job and adjusted version parsing for mainline Debian
.github/workflows/alpine-stable.yml Added OTEL build job and adjusted version parsing for stable Alpine
.github/workflows/alpine-mainline.yml Added OTEL build job and adjusted version parsing for mainline Alpine
Comments suppressed due to low confidence (4)

update.sh:33

  • [nitpick] The local associative array otel shadows the global OTEL version map. Consider renaming the local variable to otel_pkg or similar to avoid confusion.
declare -A otel=(

.github/workflows/debian-stable.yml:255

  • [nitpick] The new OTEL job duplicates much of the core workflow. Consider extracting common steps into a reusable workflow or using YAML anchors to reduce duplication.
  otel:

update.sh:32

  • [nitpick] Add a brief comment explaining the purpose of the otel associative array and how it's used in package generation for future maintainers.
# Current otel versions

update.sh:127

  • The loop for p in $otel will run once with an empty string if no OTEL package is set. Wrap this loop in an if [ -n "$otel" ] guard to prevent unintended empty iterations.
for p in $otel; do

@hangy
Copy link
Author

hangy commented Jun 16, 2025

can you also update https://github.com/nginx/docker-nginx-unprivileged/pull/318/files#diff-6456e6e208552591349a09da1db573bb5de5fdfe76776686baf7558f547f4287L31 to grep the eight iteration of mainline (grep -m7 to grep -m8), and similarly, change the Debian workflow to grep the seventh iteration of mainline? The GH workflow won't work correctly otherwise.

Sorry, hadn't noticed that grep dependency 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Open Telemetry enabled Images ;)
2 participants