Decide on the specification for key names for log correlation

[sebright]

This issue is for continuing the discussion about key names used in log correlation from #181 (comment) .

/cc @rakyll @Ramonza @adriancole @bogdandrutu

[codefromthecrypt]

maybe add to the description the current state and the proposed one?

[sebright]

I was planning to add the current state after I merge #181, since the key specification could still change. I didn't want to forget to open an issue, though.

[sebright]

Here is the specification for key names after #181 was merged, though the whole specification for log correlation is currently labeled as a draft:

Key names for tracing data

Some logging frameworks allow the insertion of arbitrary key-value pairs into log entries. When
a log correlation implementation inserts tracing data by that method, the key names should be
"opencensusTraceId", "opencensusSpanId", and "opencensusTraceSampled" by default. The log
correlation implementation may allow the user to override the tracing data key names.

[sebright]

There were two main suggested changes in #181 (comment):

• Use shorter key names.
• Allow each implementation to use a format that is consistent with the language/logging framework that it supports.

[yurishkuro]

What information is the opencensus prefix trying to add? If you call them just traceId/spanId, would it not cover the needs of most people?

[sebright]

I thought that it might be better to add "opencensus" to avoid conflicting with key-value pairs that other libraries insert into the log entry or logging context. However, conflicts probably won't be an issue if all log correlation implementations also provide a way to override the key names when necessary.

[yurishkuro]

It seems there's little chance of having another tracing lib and OpenCensus in the same application, so chance of clashes in one service are negligible. Across services - sure, but then if they interoperate on tracing then you'd still expect trace id to be the same (otherwise what's the point).

Better to allow overriding the fields for rare scenarios and default them to most intuitive names for everyone else.

[codefromthecrypt]

I think multiple tracing libraries are actually quite likely. Some tools have tracing at very low levels (even pre 1.0 stuff). However, I do agree that intuitive is nice regardless of this... I also prefer traceId, spanId actually under the same logic you mention. for why parentId? there still exist log based solutions which parse spans. one example is cloud foundry, but there were others prior to that too. personally I would like parentId to be opt-in ex no overhead unless requested.

[sebright]

I made a PR for shorter key names: #199

[sebright]

I merged #199, so the current key names are "traceId", "spanId", and "traceSampled".

[sebright]

Does anyone object to removing "draft" from the log correlation spec now that it uses the short key names? I would prefer to continue specifying the exact key names, since I think it is simpler than allowing multiple key formats or specifying different keys for different languages.

There are still several TODOs for features such as parent span ID but I think those could be added later without any backwards compatibility issues.

[codefromthecrypt]

in general do we have a gate for removing draft? like a few libraries supporting it?

[sebright]

Step 3 in https://github.com/census-instrumentation/opencensus-specs/blob/master/Process.md#step-3-specs-pull-request recommends creating a pull request implementing the change in at least one language, though I don't see any guidelines on testing the feature in a released library.

/cc @bogdandrutu

[codefromthecrypt]

hmmm proposing a single pull request seems a really low barrier though likely you could guess my opinion.

…

On Tue, 16 Oct 2018, 12:52 sebright, ***@***.***> wrote: Step 3 in https://github.com/census-instrumentation/opencensus-specs/blob/master/Process.md#step-3-specs-pull-request recommends creating a pull request implementing the change in at least one language, though I don't see any guidelines on testing the feature in a released library. /cc @bogdandrutu <https://github.com/bogdandrutu> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#195 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD619WDH9Q0RkB09cB-ynHd1xRzsmIEks5ulWYagaJpZM4XN24o> .

[c24t]

For the moment we have only Java and Go here. Can we get some input from other languages?

FWIW I don't see any problem using these names in python since they don't overlap with the built in LogRecord attributes. This is assuming we'll pass the context info to the logger with the extra kwarg.

And camelCase may be unconventional in python, but not so unconventional that the logging library doesn't use it itself!

[sebright]

I opened an issue for requiring each new feature to be implemented in a few libraries: #202. So far, log correlation has been implemented in two libraries, https://github.com/census-instrumentation/opencensus-java/tree/master/contrib/log_correlation/log4j2 and https://github.com/census-instrumentation/opencensus-java/tree/master/contrib/log_correlation/stackdriver.

[sebright]

Closing, since I updated the key names in #199 and finalized the spec in #203.