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

Set hostname and trace analytics via JSON config #110

Closed
cgilmour opened this issue Aug 28, 2019 · 3 comments · Fixed by #123
Closed

Set hostname and trace analytics via JSON config #110

cgilmour opened this issue Aug 28, 2019 · 3 comments · Fixed by #123
Assignees
Milestone

Comments

@cgilmour
Copy link
Contributor

This library is used by ingress-nginx, which uses nginx-opentracing to load the plugin, and ingress-nginx generates a JSON file to configure the tracer.

Most settings are supported via JSON, but some newer ones are not.
The reason during development was because they affect a different path in the codebase - its internal WritingSpanBuffer - and the target during development didn't load config via JSON.

The inconsistency has affected a developer that wanted to add these new features to ingress-nginx as options the user can set, because it couldn't add them to the generated JSON to set them.
While there are other ways to make it work, it's hard to explain to users and is a bit hacky.

Related:

@cgilmour cgilmour added this to the 1.1.0 milestone Aug 28, 2019
@cgilmour cgilmour self-assigned this Aug 28, 2019
@ellieayla
Copy link

@cgilmour what happened with this? The 1.1.0 milestone looks weird.

@cgilmour
Copy link
Contributor Author

cgilmour commented Jan 6, 2020

Sorry @alanjcastonguay this didn't get implemented yet, and I also didn't update about it in comments.

I'll aim to get it completed by the end of this week.

@cgilmour cgilmour modified the milestones: 1.1.0, 1.1.2, 1.1.3 Jan 6, 2020
@cgilmour
Copy link
Contributor Author

Just to give an update: I've been working on this today, and it'll be part of a bundle of fixes we intend to release this week.

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 a pull request may close this issue.

2 participants