Support Jaeger process tags #76
Conversation
exporter/jaeger/jaeger.go
Outdated
| @@ -14,18 +16,45 @@ func init() { | |||
| }) | |||
| } | |||
|
|
|||
| func getProcessTags(cfg *opencensus.Config) []jaeger.Tag { | |||
There was a problem hiding this comment.
I'd rather return an error, so the function does not need to panic when something goes wrong
There was a problem hiding this comment.
I don't think that silent discard of Jaeger exporter functionality on parse failure is expected behavior.
There was a problem hiding this comment.
I also think it's not good to discard invalid tags or to simply log an error and continue without Jaeger exporter.
There was a problem hiding this comment.
Process exit on parse failure sounds like the best option. WDYT? @kpacha
|
@rofleksey thanks for your contribution! Can you check my comment regarding avoiding panics? |
|
@kpacha Can you please check my comments above? |
|
In my opinion, failing exporters should not stop the execution of the gateway. The errors should be logged but that's it. The single doubt I have so far is if the tags are required to init the exporter or not (if tag initialization failed, should we abort the exporter or not?). I think we should revisit the process of loading of the exporters so the system logger could be injected but it will require another PR (I think I already have a proper PoC for that but I'd like to invest some more time on that). For now, we should avoid panics and just populate the error returned by the Cheers! |
|
Thanks for the feedback! I've changed panics to errors. |
|
@kpacha Can you please check if PR is fine now? Is it possible to merge it? |
|
Hello, Thank you for working on this pull request. KrakenD has traditionally offered its telemetry integration through this OpenCensus component, which has provided reliable service for over six years, but it is now transitioning to the more modern and robust OpenTelemetry framework. Due to a change in the industry, the OpenCensus integration is no longer maintained, and all efforts are focused on OpenTelemetry. KrakenD shifted to OTEL too. While the last word on this ongoing feature is for @kpacha , there will be no future development on this library, and ultimately, the repository will be archived. Issues found with OTEL can be opened in the repository krakend-otel Thank you |
No description provided.