Working OpenTelemetry sidecar (base nginx image) #8719
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
@Tobrek: This issue is currently awaiting triage. If Ingress contributors determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @Tobrek. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
needs-kind
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
images/nginx/rootfs/Dockerfile
Outdated
| @@ -71,8 +69,10 @@ RUN chmod +x /usr/local/entrypoint.sh \ | |||
| chown -R www-data.www-data ${dir}; \ | |||
| done' | |||
|
|
|||
| EXPOSE 80 443 | |||
| # enable sidecar / init cotnainer library path | |||
| RUN echo "/lib:/usr/lib:/usr/local/lib:/modules_mount/usr/lib" > /etc/ld-musl-x86_64.path | |||
There was a problem hiding this comment.
Hi Daniel, I had completely forgotten in the meeting yesterday so sorry for this late update.
I had moved he LD_LIBRARY_PATH variable from the base nginx image to the Controller Image. Please see this line
ingress-nginx/rootfs/Dockerfile
Line 34 in 7806159
/etc/ld-musl-x86_64.path file, from here to the Controller Image. So kindly remove the line here for now.
Then you can add the change to https://github.com/kubernetes/ingress-nginx/blob/7806159b386bf34b3ef0d8f130e66ad0247903af/rootfs/Dockerfile. Or if you prefer, then please let me know and I can take care of adding the file to the Controller Image, in a different PR.
|
/assign @tao12345666333 @rikatz We are reverting the changes made to the base nginx image for Opentelemetry. These changes were discussed with the maintainer of OT @esigo as well. The other open PR on OT #8622, will become relevant, once this PR is merged. @rikatz @tao12345666333 if you approve this now, we will have the base image promoted by tomorrow, so can talk about this in the community meeting with OTEL maintainer on this + zipkin + jaeger |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: longwuyuan, strongjz, Tobrek The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
@Tobrek there are many PRs some closed, some merged, some still open. Is it possible to use opentelemetry with the ingress? If it is would you be able to provide an example that can be added to the docs? |
|
Hallo @thomaschaaf, Sorry, without PR #8735 the opentelemetry is not really able to use. |
* Delete entrypoint script and add sidecar lib path * make otel libs path otel specific * add description * remove library path adaption from nginx base image
What this PR does / why we need it:
This PR is a separation of the PR #8622 to prepare the change base nginx image (on request of @longwuyuan ).
fixes #8437 and #5883
Types of changes
Which issue/s this PR fixes
How Has This Been Tested?
Checklist: