opentracing / specification

A place to document (and discuss) the OpenTracing specification. 🛑 This project is DEPRECATED! https://github.com/opentracing/specification/issues/163
http://opentracing.io/spec
Apache License 2.0
1.17k stars 182 forks source link

Add log level as a log key #62

Closed wu-sheng closed 7 years ago

wu-sheng commented 7 years ago

Relate to https://github.com/opentracing/specification/issues/57.

As we discussed a lot. I think we should add a log priority, to give the tracer implementations more chances to choose, when these are too many logs.

bhs commented 7 years ago

I'm a -1 on this, at least for now, though I'm willing to be convinced otherwise. :)

My concern is the difficulty we will have in mapping priority numbers across different sources onto a common semantic model. E.g., at google, higher numbers meant less important... I'm not saying google was right, but just that there's such a diversity of interpretations for "log level" as an int that we can't even expect agreement about monotonicity :)

My two cents.

yurishkuro commented 7 years ago

I'm also -1. There has been a lot of push back on OT having a logging-like API at all, and introducing log levels is going to make it even more controversial. We have a non-standard mechanism of passing log level via log fields, I would be interested to see some implementation actually making use of it first before we consider making it part of the standard.

Cf: https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)

wu-sheng commented 7 years ago

Logging-like API is controversial, I know that. I can leave the Pr open, wait to see where log goes.

tahini commented 7 years ago

Thanks for proposing this wu-sheng. I see that kind of change needs more documentation and convincing. I'll look more into actual implementations, especially the one for java, the language I mostly use these days. I'm a bit worried about the overhead of using OT. It should be very minimal when not tracing, so I'll do some benchmarks of our current OT-ish approach and actual OT

tedsuo commented 7 years ago

It's not clear why log level needs to be it's own parameter, standardizing on a log.severity key seems like enough.

wu-sheng commented 7 years ago

@tedsuo and @jmacd, I have a doubt about the word severity, but since English is not my native language, this is only my understanding. Severity means bad things happened, but in tracing, log do not equal error or bug, so severity is not right all the time.

And use a log key instead of parameter, to avoid breaking forward compatible,is a good idea.

yurishkuro commented 7 years ago

The common term is "severity level" (https://en.wikipedia.org/wiki/Syslog#Severity_level), and in the logging APIs it's most frequently referred to simply as "Level". So I think the most natural name for the log field key is "level". The "log." prefix is redundant.

As for the recommended values, I would keep it simple at the most common set: debug, info, warn, error.

Having said that, I am still -1 on putting this in the spec. I would first like to see (a) some examples where it's actually needed, and (b) a couple of mainstream tracing system having any sort of support for such field.

As a point of interest, our Hot R.O.D. example plugs into the regular logging framework and forks each log statement into the current span. The logging framework we use (Zap) supports levels, so when the logs are written to the Span we add a level field. However, that's as far as it goes, you can see the field value in the UI, but there's no special processing for it.

jmacd commented 7 years ago

@wu-sheng I would still +1 the idea of a log.level in the spec, but not a numeric value, an optional but standard string (e.g., "WARNING", "SEVERE", "FATAL"). @yurishkuro doesn't level interfere with log argument named level? The log. prefix says this is the logging level, not some other kind of level.

wu-sheng commented 7 years ago

+1 on remove log. prefix, as this is already a log-key.

wu-sheng commented 7 years ago

@jmacd Already change the type to string, and give six levels.

tahini commented 7 years ago

I think 4 levels is not enough for a tracing API. The tracers I know and use have more than that: JUL has 7 levels (https://docs.oracle.com/javase/7/docs/api/java/util/logging/Level.html). LTTng-UST has even more log levels for some specifics as well (http://lttng.org/man/3/lttng-ust/v2.9/#doc-tracepoint-loglevel).

And typically, the levels we use when "tracing" are info and lower, ie when we want more information than what is habitually obtained by simple logging.

wu-sheng commented 7 years ago

I think four levels are enough, and easy to use. After all, should not log everything in span, it is tracing system, not a log collector system.

bhs commented 7 years ago

After all, should not log everything in span, it is tracing system, not a log collector system.

This always confuses and/or angers people when I say it, but IMO "tracing" is really just "logging for distributed systems". They're both fundamentally about events (as opposed to statistics, that is). My worldview is that (transaction) traces should contain a superset of all "logs" about transactions. This excludes things like logging at process startup, etc, as those logs are non-transactional... but I think every logging statement about a request should be available to tracing systems, even if they choose to drop them.

bhs commented 7 years ago

Oh: and I agree with @yurishkuro that this proposal should be deferred until the conventions about logging in tracing systems are more well-established. There are extreme views about logging on all sides of the debate (my own extreme views included!) and this simply feels premature to me.

Not to say that it's a "bad" (or good) idea to add log priority... just that it seems premature.

wu-sheng commented 7 years ago

@bhs

This always confuses and/or angers people when I say it, but IMO "tracing" is really just "logging for distributed systems".

Can't logging everything means in performance way, logs are too many for tracing system, either in storage or network. So, I do not argue about tracing equals logging, or not. :)

As I said, I fully understand the controversial status, do not intend to merge this in short term. Tag is more like a structured data, easier to understand and process in endpoint, Log is opposite, but has more details in it.

tahini commented 7 years ago

Can't log everything means in performance way, logs are too many for tracing system,

So what does "tracing" in OpenTracing API mean? Is it only tracing a transaction across threads/machines? Like "tracking"?

Because when tracing, you can potentially have a lot lot more messages than when logging. Hence performance [when tracing or not] is one important requirement for tracing. Especially for code that is meant to stay in the code base (not possible to remove at compile-time for instance).

From my impression, OpenTracing API is meant to be used 1- to track a request between components of various kinds and 2- to put some tracer-agnostic tracepoints in the code so that one can plug any tracer behind it.

But for 2- one definitely needs to be able to log all trace events (as in every single one of the 5K calls to routine A of the transaction) in an efficient way.

But I may be wrong in my understanding of the scope of the OpenTracing API?

jmacd commented 7 years ago

https://en.wikipedia.org/wiki/Syslog#Severity_level

yurishkuro commented 7 years ago

@wu-sheng I made a note on issue 57 for people to see the additional discussion in this PR, but I would like to close this PR as it is not likely to be merged any time soon and the change is very small and can be easily made again once the decision on the main issue is reached.

wu-sheng commented 7 years ago

Sure, that is fine for me. Closing this.

cce commented 7 years ago

Late comment, to follow up on @tahini's question "what does 'tracing' mean?" — I really liked Peter Bourgon's blog post and diagram on this. By his definitions: Tracing provides a way to propagate request scope; Logging reports events; Metrics aggregate/reduce events along named dimensions. Thus at a high level, you could say the core capability of a tracing system is to provide a request scope that can be attributed to both events (e.g. spans, logs, errors) and metrics. image