Added Tag Configuration Support Lightstep Tracing #4175
Conversation
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
midnightconman
force-pushed
the
midnightconman/ls-tracing-tags-20210504
branch
from
8af7d99
to
c2cb109
Compare
|
I'm not sure if this test is flakey or if this is a real issue... it seems the tests failed at reloading? |
|
Yep, this manifest is missing from quay: I think this PR broke it: #4171 |
| // - comma separated list of key=value | ||
| // - value can be specified using the notation ${envVar:defaultValue}, where `envVar` | ||
| // is an environment variable and `defaultValue` is the value to use in case the env var is not set. | ||
| func parseTags(sTags string) opentracing.Tags { |
There was a problem hiding this comment.
Can we have a quick unit-test for it? (:
There was a problem hiding this comment.
I was thinking about that 😜 But there aren't any other unit tests in the tracing package (except stackdriver), so I figured it didn't make sense to break that ground for this simple change.
Should we add in a testing file and scaffolding for this?
There was a problem hiding this comment.
Scratch all that... I went ahead and added testing. PTAL
There was a problem hiding this comment.
Haaa, yea, something pragmatic would work, e.g just testing parsting tags (: Like what you did! Thanks!
|
Can you rebase? #4202 was merged 🤗 |
Signed-off-by: midnightconman <midnightconman@gmail.com>
Signed-off-by: midnightconman <midnightconman@gmail.com>
Signed-off-by: midnightconman <midnightconman@gmail.com>
midnightconman
force-pushed
the
midnightconman/ls-tracing-tags-20210504
branch
from
7d23dda
to
4af605c
Compare
Signed-off-by: midnightconman <midnightconman@gmail.com>
midnightconman
force-pushed
the
midnightconman/ls-tracing-tags-20210504
branch
from
434e1b8
to
57e3e01
Compare
| // - comma separated list of key=value | ||
| // - value can be specified using the notation ${envVar:defaultValue}, where `envVar` | ||
| // is an environment variable and `defaultValue` is the value to use in case the env var is not set. | ||
| func parseTags(sTags string) opentracing.Tags { |
There was a problem hiding this comment.
Haaa, yea, something pragmatic would work, e.g just testing parsting tags (: Like what you did! Thanks!
|
|
||
| testingData := []testData{ | ||
| { | ||
| description: `A very simple case, key "foo" and value "bar"`, |
|
|
||
| os.Setenv("_TEST_PARSE_TAGS", "env-bar") | ||
| for _, test := range testingData { | ||
| t.Logf("testing %s\n", test.description) |
There was a problem hiding this comment.
This is not a biggy, but we can use t.Run for consistency instead this (: But that's fine for now, for this PR, just let's remember for next one
Signed-off-by: midnightconman midnightconman@gmail.com
Changes
Fixes: #4148
Adds support to the configuration of the lightstep tracing client to send arbitrary tags to lightstep. The interface matches the interface for the Jaeger tracing config (comma-delimited string with key value pairs delimited by
=, which also supports environment variables). An example would be:Verification
This was tested via
make test.