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

Update nginx-opentracing and jaeger-client-cpp #1883

Merged
merged 8 commits into from
Aug 25, 2021

Conversation

MatyRi
Copy link
Contributor

@MatyRi MatyRi commented Aug 19, 2021

Proposed changes

Docker opentracing build stage does not yield working image due to the Opentracing library version mismatch. This change updates Nginx Opentracing library from 0.10.0 to the latest available version of 0.19.0.
It also requires higher version of Jaeger Tracing module which can't be just downloaded (last available download is v0.4.2) and therefore Jaeger Tracing module v0.7.0 is now built as part of the opentracing-builder stage. Latest Jaeger Tracing also has a dependency on libyaml libraries which are copied during the final build stage.

Changes just affect the Open Source version of Nginx Ingress

It seems the smoke tests in Github pipeline do not test the actual loading of the opentracing library in nginx config. It would be great to add this to the tests as well but I don't know how to do that right now, so I'm just sharing these changes which I tested.

Changes in the Dockerfile reflect the build steps in here: https://github.com/opentracing-contrib/nginx-opentracing/blob/master/Dockerfile

Checklist

  • [ x ] I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • [ x ] I have checked that all unit tests pass after adding my changes
  • [ x ] I have updated necessary documentation
  • [ x ] I have rebased my branch onto master
  • [ x ] I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

Docker opentracing build stage does not yield working image due to the Opentracing library version mismatch. Update Nginx Opentracing library from 0.10.0 to the latest available version of 0.19.0. 
Change also requires higher version of Jaeger Tracing module which can't be just downloaded (last available download is v0.4.2) and therefore build Jaeger Tracing module v0.7.0 as part of the opentracing-builder stage.
Keep the plus version untouched.
@soneillf5
Copy link
Contributor

Hi @MatyRi,

Thanks for creating this change, we'll take a look and review it as soon as we can.

Sean

Copy link
Member

@ciarams87 ciarams87 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for doing this!

build/Dockerfile Outdated Show resolved Hide resolved
Copy link
Member

@lucacome lucacome left a comment

Choose a reason for hiding this comment

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

Thanks @MatyRi! I'll merge it once the tests pass 🚀

@lucacome lucacome changed the title Opentracing update Update nginx-opentracing and jaeger-client-cpp Aug 25, 2021
@lucacome lucacome merged commit 9a0e69f into nginxinc:master Aug 25, 2021
@ciarams87 ciarams87 added the enhancement Pull requests for new features/feature enhancements label Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants