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

adding OTEL module #14

Merged
merged 10 commits into from
Sep 9, 2024
Merged

adding OTEL module #14

merged 10 commits into from
Sep 9, 2024

Conversation

SpeedyH30
Copy link
Contributor

Here is a working build, though bash can be removed as it was added for testing. along with curl in the runDeps
I don't think its the cleanest way of this working but nginx haven't made it very nice to work or add this module, but its something to work from. I've built it and had it pushing into a jaeger container, fancy index is still working along with other modules

@byjg
Copy link
Owner

byjg commented Sep 6, 2024

Sorry for my delay in taking a look on it. I was trying to fix the build for forked repositories. Now, the build is fixed.

The only thing I am not confident in accept is the conf/nginx.conf change. It is too specific and could break other people's projects.

I understand that is a nice to have, but that configuration can be in the documentation.

Also, you can inject this code by several ways:

  • Build a docker image from the nginx-extras base and add this file
  • Inject in runtime by defining a volume in the docker command line or docker compose
  • If you are using Kubernetes you can inject in runtime by using a config map.

So, if you remove the conf/nginx.conf and (optionally) add a documentation I can merge it right away and create a version tag for it.

@SpeedyH30
Copy link
Contributor Author

SpeedyH30 commented Sep 6, 2024 via email

@byjg byjg merged commit e0ad156 into byjg:master Sep 9, 2024
2 checks passed
@byjg
Copy link
Owner

byjg commented Sep 9, 2024

Release 1.26 published

docker pull byjg/nginx-extras:1.26

Thank you.

@byjg byjg mentioned this pull request Sep 9, 2024
Closed
@SpeedyH30
Copy link
Contributor Author

Thanks very much

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