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

Suport for collector level tags #1844

Closed
pavolloffay opened this issue Oct 9, 2019 · 5 comments · Fixed by #1854
Closed

Suport for collector level tags #1844

pavolloffay opened this issue Oct 9, 2019 · 5 comments · Fixed by #1854

Comments

@pavolloffay
Copy link
Member

Similar to agent tags #833
Related to removing duplicated tags in collector #1778

Add the ability to the collector to add a set of tags to incoming spans.

The agent supports JAEGER_TAGS for adding tags, however if we support the same env var in collector the same set of tags could be added twice - although it depends if we remove duplicate tags in #1778.

Use cases:

  • add tenant information cc) @radekg
@yurishkuro
Copy link
Member

if we do this, let's make sure we reuse the code from the agent

radekg added a commit to Klarrio/jaeger that referenced this issue Oct 9, 2019
@radekg
Copy link
Contributor

radekg commented Oct 9, 2019

@yurishkuro, would the referenced commit be close to what you'd imagine such a feature to look like? It would obviously require tests. For clarity, this is not a PR yet.

@yurishkuro
Copy link
Member

@radekg not quite, because it adds the tags unconditionally, and there's a requirement to fix that (e.g. #1788)

@radekg
Copy link
Contributor

radekg commented Oct 10, 2019

@yurishkuro I've opened the PR so the work can be tracked. I'll add tests in the next couple of days. Regarding #1788, I wonder what's the best way to proceed. Probably wait for that to be merged first and then adapt my PR?

Edit: alternative would be to have this merged with unit tests in and then adapt #1788 to take collector tags into account in a single swipe.

@yurishkuro
Copy link
Member

Yes, you're probably right, we can merge #1854 first, and then add the deduping policy as a separate ticket.

radekg added a commit to Klarrio/jaeger that referenced this issue Oct 11, 2019
Signed-off-by: radekg <radek@gruchalski.com>
radekg added a commit to Klarrio/jaeger that referenced this issue Oct 28, 2019
Signed-off-by: radekg <radek@gruchalski.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants