Add traceId to logging exporter #1246
Conversation
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
pavolloffay
requested review from
anuraaga,
iNikem,
jkwatson,
trask and
tylerbenson
as code owners
...ters/logging/src/main/java/io/opentelemetry/javaagent/exporters/logging/LoggingExporter.java
Outdated
Show resolved
Hide resolved
...ters/logging/src/main/java/io/opentelemetry/javaagent/exporters/logging/LoggingExporter.java
Outdated
Show resolved
Hide resolved
| @@ -33,7 +33,7 @@ public LoggingExporter(String prefix) { | |||
| @Override | |||
| public CompletableResultCode export(Collection<SpanData> list) { | |||
| for (SpanData span : list) { | |||
| System.out.print(prefix + " " + span.getName() + " " + span.getSpanId() + " "); | |||
| System.out.printf("%s %s %s %s", prefix, span.getName(), span.getTraceId(), span.getSpanId()); | |||
There was a problem hiding this comment.
printf is sloooow - can we switch back to appending?
There was a problem hiding this comment.
Also looks like we're missing space between span ID and attributes now probably good to have that (or maybe enclose attributes in {}
There was a problem hiding this comment.
No objection to switching back to appending. Good catch on the missing space 👍
There was a problem hiding this comment.
@anuraaga Any numbers to back that claim? :) And is it important enough for us?
There was a problem hiding this comment.
Have benchmarked similar now and again, generally 10-20x slower. The readability difference is small enough to get the free speed, and I always recommend people to just never use printf in Java, unless formatting numbers, since it's not worth the cognitive load of deciding between one or the other the performance difference is too great. So I'd consider it important for us.
There was a problem hiding this comment.
I will submit a PR to use the appending.
Would it be better to use string builder
Signed-off-by: Pavol Loffay ploffay@redhat.com
Resolves #1245