Feature request: Set static tags through env variables about kong-plugin-zipkin

[jcchavezs]

This is a typical request. Having defaultTags like aws.instance is a very common use case so +1 on this.

[qvistgaard]

I agree, can we please get custom tags. We have multiple k8s clusters, it would be nice to have that information in the span

[seh]

I was going to file a new issue, but I think this one captures what I'm looking for. I'd like to be able to configure a map of field/tag/label keys and values in my KongPlugin object for the Zipkin plugin, and have Kong export those values as part of every top-level trace span. It would be nice if they were included in every span within the trace, but I'd settle for top-level only if need be.

Is anyone working on this now?

[jcchavezs]

Not that I know @seh. Up for a PR?

[seh]

I can read Lua. Writing some may be possible. It would require a lot of patient coaching during review.

The question is whether the maintainers are willing to accept such a feature, should it arrive as a patch. We'd have to settle some design points, such as:

• Wnhat happens if the configuration nominates a statically defined value for a tag that's normally exported by the plugin (e.g. "http.method").
• Would we try to catch these when validating the configuration?
• Would we underlay the static tags beneath the normal ones, so that we'd clobber the static tags should such a collision arise?
• Are there syntactic restrictions on the tag names and values?

[hbagdi]

Worker processes do not receive environment variables that you pass to Kong at startup.
See: https://nginx.org/en/docs/ngx_core_module.html#env

Is configuring tags via the plugin config not an option here? (I'm not a zipking/tracing superuser)

[seh]

For my purposes, reading environment variables could be convenient, but it's not essential. I'd settle for static plugin configuration. It's more about what you're allowed to configure there.

[hbagdi]

@kikito can probably help drive the conversation here.

My 2 cents would be that static tags have lower precedence over the dynamic ones that are injected by the plugins. We shouldn't do any validation because such validation can get out of date as the plugins code receives more love. The validation on the tag key and value should be same as what is accepted in the tracing standards. If standards are vague, we should start narrow and small.

[seh]

Can you clarify what "narrow and small" means in this context? Is that "small" as in few restrictions, being liberal in what we'll tolerate, or "small" as in only tolerating a few things with the possibility of relaxing the restrictions later?

[hbagdi]

or "small" as in only tolerating a few things with the possibility of relaxing the restrictions later?

This one.

[kikito]

Just FYI I am going to start working on this.

What happens if the configuration nominates a statically defined value for a tag that's normally exported by the plugin (e.g. "http.method").

The normally exported plugin by the tag will "win". This is a purely self-serving move, since it would be easier for me to program it this way.

Would we try to catch these when validating the configuration?

It is actually not very difficult to do this. At least, a simple version of it is. Might add.

Would we underlay the static tags beneath the normal ones, so that we'd clobber the static tags should such a collision arise?

Static tags will be clobbered.

Are there syntactic restrictions on the tag names and values?

Not really. The config might live in a JSON/YAML file so that puts certain restrictions on the values though.

[seh]

That all sounds good, @kikito. I look forward to seeing your patch, learning a little more Lua through review.

[jcchavezs]

This is awesome @kikito. BTW if you mean that on a conflict, the request tag will have more priority than the static one this is the right behaviour. In general I can't think of a use case where static tags would collide with the request ones, their nature is different but in any case the specific ones (those coming from the request) should have precedence.

…

On Fri, 22 May 2020, 15:57 Enrique García Cota, ***@***.***> wrote: Just FYI I am going to start working on this. What happens if the configuration nominates a statically defined value for a tag that's normally exported by the plugin (e.g. "http.method"). The normally exported plugin by the tag will "win". This is a purely self-serving move, since it would be easier for me to program it this way. Would we try to catch these when validating the configuration? It is actually not very difficult to do this. At least, a simple version of it is. Might add. Would we underlay the static tags beneath the normal ones, so that we'd clobber the static tags should such a collision arise? Static tags will be clobbered. Are there syntactic restrictions on the tag names and values? Not really. The config might live in a JSON/YAML file so that puts certain restrictions on the values though. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXOYAT6DB6ZP5YTKNYV3RLRS2ADTANCNFSM4E6IY4RQ> .

[kikito]

Ok the PR is ready in #84, and the tests are green. Will leave it open for a couple days in case someone wants to give it a look before merging it.

[kikito]

Implemented in #84, will be released in the next version of the plugin.