Add LoggerName to Log Data Model #1236

tigrannajaryan · 2020-11-18T14:56:59Z

Resolves #1215

It appears that logger name is sufficiently different to warrant a new concept in
Otel logs. For detailed discussion see the link above.

bogdandrutu · 2020-11-18T15:41:33Z

specification/logs/data-model.md

@@ -434,7 +442,8 @@ Example 1
  "SpanId": "43222c2d51a7abe3",
  "SeverityText": "INFO",
  "SeverityNumber": 9,
-  "Body": "20200415T072306-0700 INFO I like donuts"
+  "Body": "20200415T072306-0700 INFO I like donuts",
+  "LoggerName": "com.example.App.MyClass"


Is this not the "InstrumentationName"?

See discussion #1215
The consensus is that InstrumentationName is different from LoggerName.

If this is the case we definitely failed with InstrumentationName and we should probably remove that. This is hitting again and again.

@Oberon00 @arminru as people who pushed on the InstrumentationName which creates again more confusion.

I would think that in this case InstrumentationName would be the instrumentation or adapter that is being used to collect these events - something like "Log4J". LoggerName is determined by the author of the code being instrumented, not the instrumenting code. In Java it might be a package and class name, in other languages it might be "database_access", or "database::access", or any number of schemes. The commonality is that it's determined without OpenTelemetry necessarily in mind, but is rather following the conventions of the language or framework or application developer rather than our conventions.

Another way to cut it would be in cardinality- for a Java application you could adapt both Log4J and Logback. Hopefully from the exporter events from each would be indistinguishable from the other, but each will contain many (almost always at least hundreds) of logger names.

A Spring application using Log4J will have logging events delivered from application code via Log4J and events from the Spring framework via Logback. Both types of events should be grouped in a containing trace. We may also want to see which ones came via one route or the other. I think that both are important, but distinct.

@zenmoto "Log4J" will be the telemetry.sdk.name. So definitely the name of the library has it's own place.

LoggerName is determined by the author of the code being instrumented, not the instrumenting code.

It is the same for metrics and traces, but it depends who is the author of the "code". If the logging happens via a plugin or is embedded in the code itself.

Also the APIs that we right now have for getting a tracer (see https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#get-a-tracer) is exactly like the "GetLogger", I see no difference.

Let's save I've read this thread and agree that we should have a new field. Is there a name that would make this concept less logging specific? There are ways that "LoggerName" is like a resource, and if so we could establish a semantic convention for "LoggerName". However, we've resisted adding non-process-identifying resource attributes, as far as I know, and LoggerName is scoped to part of a process.

I've advocated for "resource scopes" in the past: open-telemetry/oteps#78

In that proposal, we might just add a resource-scoped "logger.name=<your_class_name_or_whatever>" attribute that would apply to all (spans, logs, metrics) when used.

@jmacd I agree on several points of your proposal for resource scopes. However, I believe it is orthogonal and more complex issue, which also likely requires changes to traces and metrics.

Could the "logger name" be modeled as a "scope" if we had resource scopes? Possibly. However, we don't have them and I think it is likely that we won't have them for a while (if ever). I do not want to make logs wait for something that may not be accepted.

Again, I generally like your "scopes" proposal and I think it is very useful for representing certain types of data, but I think the "logger name" can be introduced as a first-class concept independently from the scopes.

However, where it does not fit at all is when the logger name is NOT a class name but an arbitrary hierarchical name, which is totally allowed by most traditional loggers. For example, it can be even something dynamic like /customer/{customerID}.

Given these dependencies on the logger name, I am not sure InstrumentationLibrary/Component even makes sense for loggers, since a custom name like /customer/{customerID} could be reused across different layers of the application, making InstrumentationX resource attribute meaningless.

@yurishkuro since you agree that the "logger name" does not belong to the InstrumentationLibrary.Name do you also agree with the suggestion that this PR makes, i.e. to introduce a new LoggerName field or you have a different proposal?

I'm ok with it in the data model.

This seems to be a logical grouping name like "log group". Vendors call it a group, folder, etc.

yurishkuro

LGTM

yurishkuro · 2020-11-27T17:37:25Z

specification/logs/data-model.md

@@ -434,7 +442,8 @@ Example 1
  "SpanId": "43222c2d51a7abe3",
  "SeverityText": "INFO",
  "SeverityNumber": 9,
-  "Body": "20200415T072306-0700 INFO I like donuts"
+  "Body": "20200415T072306-0700 INFO I like donuts",
+  "LoggerName": "com.example.App.MyClass"


I'm ok with it in the data model.

Resolves open-telemetry#1215 It appears that logger name is sufficiently different to warrant a new concept in Otel logs. For detailed discussion see the link above.

tigrannajaryan · 2020-11-27T18:31:42Z

@maxgolov ~~this PR addresses the issues that you opened. Please review.~~ Nevermind, I see that you already did.

github-actions · 2020-12-05T03:26:41Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

tigrannajaryan · 2020-12-07T17:58:20Z

I am going to remove the "Stale" label to give people more time. Most of Otel is focused on making the GA release so this can wait a bit.

github-actions · 2020-12-15T03:29:52Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2020-12-22T03:38:29Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

tigrannajaryan · 2021-01-04T21:09:48Z

Re-opening. This is not stale, it is deliberately postponed until it is time to focus on logs.

Oberon00 · 2021-01-08T09:28:32Z

Re-opening. This is not stale, it is deliberately postponed until it is time to focus on logs.

@tigrannajaryan Maybe you want to convert it to a draft for now? The the stale bot will leave it alone. Or you can mark it as after-ga (see https://github.com/open-telemetry/opentelemetry-specification/blob/master/.github/workflows/stale-pr.yaml#L15)

This is a slightly different take on open-telemetry#2203 Instead of renaming instrumentation library to instrumentation scope everywhere this PR suggests targetted editing of wording of the Trace/Meter obtaining API to allow not just instrumentation library but other instrumentation scopes to be used as a parameter. This change does not force renaming of API parameters and is not a breaking change. We consider it a clarification of usage to match the intent that we had for the "name" field. If this PR is accepted there will be a follow up PR that will suggest to rename InstrumentationLibrary* messages in OTLP proto to InstrumentationScope* message. Such a change will not be a breaking change for the OTLP wire format and is acceptable. If this PR is accepted we will also close open-telemetry#1236 since it will be no longer necessary. The logger name will be recorded as the instrumentation scope. This clarification will be a follow up PR that clarifies the behavior in https://github.com/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md

I think #2276 is better.

This is a slightly different take on open-telemetry#2203 Instead of renaming instrumentation library to instrumentation scope everywhere this PR suggests targetted editing of wording of the Trace/Meter obtaining API to allow not just instrumentation library but other instrumentation scopes to be used as a parameter. This change does not force renaming of API parameters and is not a breaking change. We consider it a clarification of usage to match the intent that we had for the "name" field. If this PR is accepted there will be a follow up PR that will suggest to rename InstrumentationLibrary* messages in OTLP proto to InstrumentationScope* message. Such a change will not be a breaking change for the OTLP wire format and is acceptable. If this PR is accepted we will also close open-telemetry#1236 since it will be no longer necessary. The logger name will be recorded as the instrumentation scope. This clarification will be a follow up PR that clarifies the behavior in https://github.com/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md

This is a slightly different take on #2203 Instead of renaming instrumentation library to instrumentation scope everywhere this PR suggests targetted editing of wording of the Trace/Meter obtaining API to allow not just instrumentation library but other instrumentation scopes to be used as a parameter. This change does not force renaming of API parameters and is not a breaking change. We consider it a clarification of usage to match the intent that we had for the "name" field. If this PR is accepted there will be a follow up PR that will suggest to rename InstrumentationLibrary* messages in OTLP proto to InstrumentationScope* message. Such a change will not be a breaking change for the OTLP wire format and is acceptable. If this PR is accepted we will also close #1236 since it will be no longer necessary. The logger name will be recorded as the instrumentation scope. This clarification will be a follow up PR that clarifies the behavior in https://github.com/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md

…2276) * Clarify that Trace/Meter are associated with Instrumentation Scope This is a slightly different take on #2203 Instead of renaming instrumentation library to instrumentation scope everywhere this PR suggests targetted editing of wording of the Trace/Meter obtaining API to allow not just instrumentation library but other instrumentation scopes to be used as a parameter. This change does not force renaming of API parameters and is not a breaking change. We consider it a clarification of usage to match the intent that we had for the "name" field. If this PR is accepted there will be a follow up PR that will suggest to rename InstrumentationLibrary* messages in OTLP proto to InstrumentationScope* message. Such a change will not be a breaking change for the OTLP wire format and is acceptable. If this PR is accepted we will also close #1236 since it will be no longer necessary. The logger name will be recorded as the instrumentation scope. This clarification will be a follow up PR that clarifies the behavior in https://github.com/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md

…2276) * Clarify that Trace/Meter are associated with Instrumentation Scope This is a slightly different take on open-telemetry/opentelemetry-specification#2203 Instead of renaming instrumentation library to instrumentation scope everywhere this PR suggests targetted editing of wording of the Trace/Meter obtaining API to allow not just instrumentation library but other instrumentation scopes to be used as a parameter. This change does not force renaming of API parameters and is not a breaking change. We consider it a clarification of usage to match the intent that we had for the "name" field. If this PR is accepted there will be a follow up PR that will suggest to rename InstrumentationLibrary* messages in OTLP proto to InstrumentationScope* message. Such a change will not be a breaking change for the OTLP wire format and is acceptable. If this PR is accepted we will also close open-telemetry/opentelemetry-specification#1236 since it will be no longer necessary. The logger name will be recorded as the instrumentation scope. This clarification will be a follow up PR that clarifies the behavior in https://github.com/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md

tigrannajaryan requested review from a team, maxgolov, kumoroku and zenmoto November 18, 2020 14:56

github-actions bot assigned yurishkuro Nov 18, 2020

bogdandrutu previously requested changes Nov 18, 2020

View reviewed changes

zenmoto approved these changes Nov 18, 2020

View reviewed changes

kumoroku approved these changes Nov 20, 2020

View reviewed changes

yurishkuro approved these changes Nov 27, 2020

View reviewed changes

Add LoggerName to Log Data Model

16d0213

Resolves open-telemetry#1215 It appears that logger name is sufficiently different to warrant a new concept in Otel logs. For detailed discussion see the link above.

tigrannajaryan force-pushed the feature/tigran/logger-name branch from 568c562 to 16d0213 Compare November 27, 2020 18:29

kxyr mentioned this pull request Dec 1, 2020

Logging API and SDK modifications open-o11y/opentelemetry-cpp#12

Closed

github-actions bot added the Stale label Dec 5, 2020

tigrannajaryan removed the Stale label Dec 7, 2020

github-actions bot added the Stale label Dec 15, 2020

github-actions bot closed this Dec 22, 2020

tigrannajaryan reopened this Jan 4, 2021

github-actions bot removed the Stale label Jan 5, 2021

tigrannajaryan added the spec:logs Related to the specification/logs directory label Jan 6, 2021

pmm-sumo approved these changes Jan 6, 2021

View reviewed changes

tigrannajaryan marked this pull request as draft January 8, 2021 18:02

cijothomas approved these changes Jan 19, 2022

View reviewed changes

tigrannajaryan mentioned this pull request Jan 19, 2022

Clarify that Trace/Meter are associated with Instrumentation Scope #2276

Merged

bogdandrutu mentioned this pull request Feb 1, 2022

Ideas on how to InstrumentationLibrary -> Scope #2307

Closed

carlosalberto closed this in #2276 Feb 15, 2022

tigrannajaryan deleted the feature/tigran/logger-name branch February 15, 2022 17:42

srikanthccv mentioned this pull request Oct 29, 2022

Record logger name as the instrumentation scope name open-telemetry/opentelemetry-python#2485

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LoggerName to Log Data Model #1236

Add LoggerName to Log Data Model #1236

tigrannajaryan commented Nov 18, 2020

bogdandrutu Nov 18, 2020

tigrannajaryan Nov 18, 2020

bogdandrutu Nov 18, 2020

zenmoto Nov 18, 2020

bogdandrutu Nov 18, 2020

jmacd Nov 24, 2020

tigrannajaryan Nov 27, 2020

tigrannajaryan Nov 27, 2020

yurishkuro Nov 27, 2020

rakyll Feb 16, 2021

yurishkuro left a comment

yurishkuro Nov 27, 2020

tigrannajaryan commented Nov 27, 2020 •

edited

Loading

github-actions bot commented Dec 5, 2020

tigrannajaryan commented Dec 7, 2020

github-actions bot commented Dec 15, 2020

github-actions bot commented Dec 22, 2020

tigrannajaryan commented Jan 4, 2021

Oberon00 commented Jan 8, 2021 •

edited

Loading

Add LoggerName to Log Data Model #1236

Add LoggerName to Log Data Model #1236

Conversation

tigrannajaryan commented Nov 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurishkuro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tigrannajaryan commented Nov 27, 2020 • edited Loading

github-actions bot commented Dec 5, 2020

tigrannajaryan commented Dec 7, 2020

github-actions bot commented Dec 15, 2020

github-actions bot commented Dec 22, 2020

tigrannajaryan commented Jan 4, 2021

Oberon00 commented Jan 8, 2021 • edited Loading

tigrannajaryan commented Nov 27, 2020 •

edited

Loading

Oberon00 commented Jan 8, 2021 •

edited

Loading