-
Notifications
You must be signed in to change notification settings - Fork 486
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
Nginx autoinstrumentation image #1852
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: new_component | ||
|
||
# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action) | ||
component: operator | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: Build image with instrumentation library v1.0.3 for Nginx. | ||
|
||
# One or more tracking issues related to the change | ||
issues: [1852] | ||
|
||
# (Optional) One or more lines of additional information to render under the primary note. | ||
# These lines will be padded with 2 spaces and then inserted directly into the document. | ||
# Use pipe (|) for multiline entries. | ||
subtext: |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
name: "Publish Nginx Auto-Instrumentation Image" | ||
|
||
on: | ||
push: | ||
paths: | ||
- 'autoinstrumentation/nginx/**' | ||
- '.github/workflows/publish-autoinstrumentation-nginx.yaml' | ||
branches: | ||
- main | ||
pull_request: | ||
paths: | ||
- 'autoinstrumentation/nginx/**' | ||
- '.github/workflows/publish-autoinstrumentation-nginx.yaml' | ||
workflow_dispatch: | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
cancel-in-progress: true | ||
|
||
jobs: | ||
publish: | ||
runs-on: ubuntu-20.04 | ||
|
||
steps: | ||
- uses: actions/checkout@v3 | ||
|
||
- name: Read version | ||
run: echo "VERSION=$(cat autoinstrumentation/nginx/version.txt)" >> $GITHUB_ENV | ||
|
||
- name: Docker meta | ||
id: meta | ||
uses: docker/metadata-action@v4 | ||
with: | ||
images: ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-nginx | ||
tags: | | ||
type=match,pattern=v(.*),group=1,value=v${{ env.VERSION }} | ||
|
||
- name: Set up QEMU | ||
uses: docker/setup-qemu-action@v2 | ||
|
||
- name: Set up Docker Buildx | ||
uses: docker/setup-buildx-action@v2 | ||
|
||
- name: Cache Docker layers | ||
uses: actions/cache@v3 | ||
with: | ||
path: /tmp/.buildx-cache | ||
key: ${{ runner.os }}-buildx-${{ github.sha }} | ||
restore-keys: | | ||
${{ runner.os }}-buildx- | ||
|
||
- name: Login to GitHub Package Registry | ||
uses: docker/login-action@v2 | ||
with: | ||
registry: ghcr.io | ||
username: ${{ github.repository_owner }} | ||
password: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- name: Build and push | ||
uses: docker/build-push-action@v4 | ||
with: | ||
context: autoinstrumentation/nginx | ||
platforms: linux/amd64 | ||
push: ${{ github.event_name == 'push' }} | ||
build-args: version=${{ env.VERSION }} | ||
tags: ${{ steps.meta.outputs.tags }} | ||
labels: ${{ steps.meta.outputs.labels }} | ||
cache-from: type=local,src=/tmp/.buildx-cache | ||
cache-to: type=local,dest=/tmp/.buildx-cache |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
|
||
############################ | ||
# STEP 1 download the webserver agent | ||
############################ | ||
FROM alpine:latest as agent | ||
|
||
ARG version | ||
|
||
RUN mkdir /opt/opentelemetry | ||
WORKDIR /opt/opentelemetry | ||
|
||
RUN wget https://github.com/open-telemetry/opentelemetry-cpp-contrib/releases/download/webserver%2Fv$version/opentelemetry-webserver-sdk-x64-linux.tgz | ||
RUN mkdir agent | ||
RUN tar -xvf opentelemetry-webserver-sdk-x64-linux.tgz -C agent | ||
|
||
############################ | ||
# STEP 2 build streamlined image | ||
############################ | ||
FROM alpine:latest | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this need to be alpine? Could we change it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interesting idea - i think it has to have the busybox at least - let me give it a try. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't want to use busybox #1600 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. I need a bit more than cp - currently I'm using sh, cp, echo, and sed. Alternatively, I could build all the needed logic in Go, which would have also it's benefits other than using "from scratch". If going this way - where would that go? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if I build an image with the instrumentation libraries and a go-based utility which does all the instrumentation logic, which runs without any dependencies in |
||
|
||
COPY --from=agent /opt/opentelemetry/agent/opentelemetry-webserver-sdk /opt/opentelemetry | ||
|
||
RUN chmod a+w /opt/opentelemetry/logs | ||
|
||
CMD ["cat", "Just delivering the Opentelemetry Nginx agent"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# How to build Nginx auto-instrumentation docker image | ||
|
||
To build image for Nginx auto instrumentation, use the following commands | ||
|
||
``` | ||
export REPO_NAME="<your-docker-image-repo-name>" | ||
export IMAGE_NAME_PREFIX="autoinstrumentation-nginx" | ||
export IMAGE_VERSION=`cat version.txt` | ||
export IMAGE_NAME=${REPO_NAME}/${IMAGE_NAME_PREFIX}:${IMAGE_VERSION} | ||
docker build --build-arg version=${IMAGE_VERSION} . -t ${IMAGE_NAME} -t ${REPO_NAME}/${IMAGE_NAME_PREFIX}:latest | ||
docker push ${IMAGE_NAME} | ||
docker push ${REPO_NAME}/${IMAGE_NAME_PREFIX}:latest | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
1.0.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image seems to be similar to what we already have for apache HTTPd https://github.com/open-telemetry/opentelemetry-operator/blob/main/autoinstrumentation/apache-httpd/Dockerfile#L12
Is there any reason why we should maintain another one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about that, too, but then I thought it would be better to have them separate for versioning reasons. It looks the different versions of the Otel libraries support different versions of Nginx, keeping a common image may bring unwanted dependency in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version of each instrumentation image is still specified in
versions.txt
(the default one), via a flag on the operator and in the CR for each instrumentation.If we don't foresee packaging different libraries in the docker image it does not make sense to create two identical images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I'm going to use the current one then and if anything prevents using the same image in the future, then I'll handle it accordingly.
I'm going to close this PR and update #1853 with a comment that the apache image is used for Nginx, too.