Kong/kong-plugin-zipkin

Feature request: Set static tags through env variables

FlorianLautenschlager opened this issue · 15 comments

In our project we use some env variables to inject static tags, e.g. the environment.
Does this sound to you as a neat feature?

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

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 commented

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?

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

seh commented

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?

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 commented

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.

@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 commented

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?

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

This one.

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 commented

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

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.

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