Log traces only when these are available

[refs]

Prevent from logging traceid=0000000000000000. Now logs would appear as:

when traces are enabled, then the output looks like:

as god intended.

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[refs]

@kobergj thanks for the extra pair of eyes! I applied your suggestions, could you have another look? 🚀

[kobergj]

Looks awesome now 👍

[labkode]

@refs originally the traceid was not taken from the tracing framework, it was always visible as it was generated from a Reva middleware crafting a random UUID. With the change to the tracing framework we have now the possibility to configure a tracing target to send the traces remotely.

However we should split two functionalities:

1. trace id in the log is always useful and should be available independently if the remote tracing collection is enabled or not.
2. send traces to remote collector for trace analysis.

With this in mind I'm not in favour of dropping the traceid but rather to fix the code to make sure it is always there

[labkode]

Traceid should always be present in the logs independently if trace collector is enabled or not.

[refs]

Traceid should always be present in the logs independently if trace collector is enabled or not.

Hm, I see your argument of always having traces regardless of these are reported or not. I will update the code to reflect that. I completely misjudged the benefits of having the traces always available 👍

[ishank011]

@refs I'd propose enabling tracing by default if it has not been explicitly disabled

diff --git a/cmd/revad/runtime/runtime.go b/cmd/revad/runtime/runtime.go
index 22d55eb3..9783092c 100644
--- a/cmd/revad/runtime/runtime.go
+++ b/cmd/revad/runtime/runtime.go
@@ -311,6 +311,15 @@ func parseCoreConfOrDie(v interface{}) *coreConf {
                os.Exit(1)
        }

+       // tracing defaults to enabled if not explicitly configured
+       if v == nil {
+               c.TracingEnabled = true
+               c.TracingEndpoint = "localhost:6831"
+       } else if _, ok := v.(map[string]interface{})["tracing_enabled"]; !ok {
+               c.TracingEnabled = true
+               c.TracingEndpoint = "localhost:6831"
+       }
+
        return c
 }

[refs]

That's a way to do it. I'm currently testing what would happen if there is no sidecar agent and we have traces. I have seen it fail trying to connect to the agent's address. Once I got it working I'll update here. But out of the box seems like a good compromise, if it works 👯

[refs]

update: the exporter fails, and crashes if missconfigured. I will figure a way to preserve the traceid for the logs regardless of enabled tracing

[ishank011]

Superseded by #2515