Add initial overall structure and classes for logs sdk #1894

srikanthccv · 2021-06-05T21:27:20Z

Description

This commit adds initial structure and classes based on the OTEP https://github.com/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md#specification

Part of #1890

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

ocelotl · 2021-06-07T23:09:04Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/export/__init__.py

+from opentelemetry.sdk.logs import LogData
+
+
+class LogExportResult(enum.Enum):


Is this part of the spec?

No, I think this will be eventually added to spec. Even I can send PR to get this ExportResult included in spec if you want.

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

lzchen · 2021-06-08T15:55:53Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+    def __init__(
+        self, log_record: LogRecord, instrumentation_info: InstrumentationInfo,
+    ):
+        self.log_record = log_record


This might need to be read-only in the future. Could save for separate PR.

lzchen · 2021-06-08T16:14:22Z

Please submit the PR to the logs branch instead.

codeboten

This PR looks great! It looks like there's a few TODOs left in the code, they look like they're documentation placeholders? If so, please add the documentation, otherwise if you could open an issue to track the work required that would be great.

codeboten · 2021-06-08T18:39:47Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+
+
+class LogEmitter:
+    # TODO: Add multi_log_processor


Please add documentation as to what a log emitter is

codeboten · 2021-06-08T18:40:14Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+
+
+class LogEmitterProvider:
+    # TODO: Add multi_log_processor


Ditto to documentation here.

codeboten · 2021-06-08T18:40:48Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+            True if all the log processors flushes the logs within timeout,
+            False otherwise.
+        """
+        # TODO: multi_log_processor.force_flush


Is this TODO necessary here?

codeboten · 2021-06-08T18:40:53Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+
+    def shutdown(self):
+        """Shuts down the log processors."""
+        # TODO: multi_log_processor.shutdown


Is this TODO necessary here?

codeboten · 2021-06-08T18:40:58Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+
+        The log processors are invoked in the same order they are registered.
+        """
+        # TODO: multi_log_processor.add_log_processor.


Is this TODO necessary here?

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

codeboten · 2021-06-08T18:50:57Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+    """
+
+    @abc.abstractmethod
+    def emit(self, log_data: LogData):


It would be nice if the SpanProcessor would also have a method called emit for consistency. No action required from this comment, just a thought.

Hmm, what would emit mean in the context of span?

I would expect it to mean the same thing as in the context of logs and metrics, that we're emitting the data from the processor. Currently this is called "on_end" in the span processor.

srikanthccv · 2021-06-08T19:15:34Z

@codeboten I added a comment on the original issue on the plan #1890 (comment). These TODOs are placeholders for actual implementation.

codeboten · 2021-06-08T20:16:11Z

@codeboten I added a comment on the original issue on the plan #1890 (comment). These TODOs are placeholders for actual implementation.

Thanks for updating that issue, I think I'm still missing something. How is the following todo related to the LogEmitter?

class LogEmitter:
    # TODO: Add multi_log_processor

ocelotl · 2021-06-08T20:37:44Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+        """
+
+
+class LogEmitter:


Is this a subclass of LogProcessor?

No, I added a diagram below. I hope it helps.

ocelotl · 2021-06-08T20:39:54Z

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py

+            ),
+        )
+
+    def add_log_processor(self, log_processor: LogProcessor):


Isn't this weird? I mean, this is a method of the LogEmitterProvider class

I am not sure I understand your question. How else would we add a log processor?

srikanthccv · 2021-06-08T21:19:36Z

@codeboten I added a comment on the original issue on the plan #1890 (comment). These TODOs are placeholders for actual implementation.

Thanks for updating that issue, I think I'm still missing something. How is the following todo related to the LogEmitter?
class LogEmitter:
    # TODO: Add multi_log_processor

I tried to make a diagram. I hope it clears up some doubts. LogEmitter sits between the logging library and SDK. It will have reference to an instance of MultiLogProcessor(yet to be implemented) from provider. LogEmitter will call .emit of multi log processor which in turn calls the .emit of Simple/Batch log processor. Does it make it sense now?

codeboten · 2021-06-08T21:33:52Z

I tried to make a diagram. I hope it clears up some doubts. LogEmitter sits between the logging library and SDK. It will have reference to an instance of MultiLogProcessor(yet to be implemented) from provider. LogEmitter will call .emit of multi log processor which in turn calls the .emit of Simple/Batch log processor. Does it make it sense now?

I see, I think this is where my understanding is lacking... I was expecting the multilog processor interface to be the same as any other processor, the way a multi span processor's interface is the same as any span processor. So in this case I wouldnt expect a multi log processor change to impact the LogEmitter which is where this TODO is.

srikanthccv · 2021-06-09T02:22:57Z

Right, The TODO comment is to add multi_log_processor (which is missing as of now) so that LogEmitter can emit received the log records and push through the SDK processors and exporters. When implemented fully it would look roughly like this.

class LogEmitter:
    def __init__(
        self,
        resource: Resource,
        multi_log_processor: Union[SyncMultiLogProcessor, ConcurrenctMultiLogProcessor]
        instrumentation_info: InstrumentationInfo,
    ):
        self._resource = resource
        self._multi_log_processor = multi_log_processor
        self._instrumentation_info = instrumentation_info

    def emit(self, record: LogRecord):
        log_data = LogData(record, self._instrumentation_info)
        self._multi_log_processor.emit(log_data)

    def flush(self):
        self._multi_log_processor.force_flush()

codeboten

Thanks @lonewolf3739 for talking through my comments, I understand the plan moving forward. This is really exciting work!

…y#1894)

* Add initial overall structure and classes for logs sdk (#1894) * Add global LogEmitterProvider and convenience function get_log_emitter (#1901) * Add OTLPHandler for standard library logging module (#1903) * Add LogProcessors implementation (#1916) * Fix typos in test_handler.py (#1953) * Add support for OTLP Log exporter (#1943) * Add support for user defined attributes in OTLPHandler (#1952) * use timeout in force_flush (#2118) * use timeout in force_flush * fix lint * Update opentelemetry-sdk/src/opentelemetry/sdk/logs/export/__init__.py Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com> * fix lint Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com> * add a ConsoleExporter for logging (#2099) Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com> * Update SDK docs and Add example with OTEL collector logging (debug) exporter (#2050) * Fix exception in severity number transformation (#2208) * Fix exception with warning message transformation * Fix lint * Fix lint * fstring * Demonstrate how to set the Resource for LogEmitterProvider (#2209) * Demonstrate how to set the Resource for LogEmitterProvider Added a Resource to the logs example to make it more complete. Previously it was using the built-in Resource. Now it adds the service.name and service.instance.id attributes. The resulting emitted log records look like this: ``` Resource labels: -> telemetry.sdk.language: STRING(python) -> telemetry.sdk.name: STRING(opentelemetry) -> telemetry.sdk.version: STRING(1.5.0) -> service.name: STRING(shoppingcart) -> service.instance.id: STRING(instance-12) InstrumentationLibraryLogs #0 InstrumentationLibrary __main__ 0.1 LogRecord #0 Timestamp: 2021-10-14 18:33:43.425820928 +0000 UTC Severity: ERROR ShortName: Body: Hyderabad, we have a major problem. Trace ID: ce1577e4a703f42d569e72593ad71888 Span ID: f8908ac4258ceff6 Flags: 1 ``` * Fix linting * Use batch processor in example (#2225) * move logs to _logs (#2240) * move logs to _logs * fix lint * move log_exporter to _log_exporter as it's still experimental (#2252) Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com> Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Co-authored-by: Leighton Chen <lechen@microsoft.com> Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com> Co-authored-by: Owais Lone <owais@users.noreply.github.com>

…y#1894)

srikanthccv marked this pull request as ready for review June 5, 2021 21:31

srikanthccv requested a review from a team as a code owner June 5, 2021 21:31

srikanthccv requested review from owais, ocelotl, codeboten and lzchen and removed request for a team June 5, 2021 21:31

srikanthccv mentioned this pull request Jun 6, 2021

Add global log emitter provider #1895

Closed

ocelotl reviewed Jun 7, 2021

View reviewed changes

lzchen reviewed Jun 8, 2021

View reviewed changes

opentelemetry-sdk/src/opentelemetry/sdk/logs/__init__.py Outdated Show resolved Hide resolved

lzchen reviewed Jun 8, 2021

View reviewed changes

srikanthccv added 2 commits June 8, 2021 22:01

Add overall structure and classes

f5599a4

Review comments

c0a11d3

srikanthccv force-pushed the logs-initial-skeleton branch from b86348d to c0a11d3 Compare June 8, 2021 16:32

srikanthccv changed the base branch from experimental to logs June 8, 2021 16:32

srikanthccv added 2 commits June 8, 2021 22:04

Change body type

5f8e031

Fix lint

cf96f85

lzchen approved these changes Jun 8, 2021

View reviewed changes

codeboten suggested changes Jun 8, 2021

View reviewed changes

srikanthccv mentioned this pull request Jun 8, 2021

Add support for OpenTelemetry logging #1890

Closed

ocelotl reviewed Jun 8, 2021

View reviewed changes

codeboten approved these changes Jun 9, 2021

View reviewed changes

codeboten merged commit 69fdad9 into open-telemetry:logs Jun 9, 2021

srikanthccv deleted the logs-initial-skeleton branch September 24, 2021 08:39

lzchen pushed a commit to lzchen/opentelemetry-python that referenced this pull request Oct 15, 2021

Add initial overall structure and classes for logs sdk (open-telemetr…

57dc05e

…y#1894)

codeboten pushed a commit that referenced this pull request Oct 29, 2021

Add initial overall structure and classes for logs sdk (#1894)

06e7729

codeboten pushed a commit to codeboten/opentelemetry-python that referenced this pull request Nov 3, 2021

Add initial overall structure and classes for logs sdk (open-telemetr…

b41c6e3

…y#1894)

lzchen pushed a commit that referenced this pull request Nov 4, 2021

Add initial overall structure and classes for logs sdk (#1894)

8df19ef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial overall structure and classes for logs sdk #1894

Add initial overall structure and classes for logs sdk #1894

srikanthccv commented Jun 5, 2021

ocelotl Jun 7, 2021

srikanthccv Jun 8, 2021

lzchen Jun 8, 2021

lzchen commented Jun 8, 2021

codeboten left a comment

codeboten Jun 8, 2021

codeboten Jun 8, 2021

codeboten Jun 8, 2021

codeboten Jun 8, 2021

codeboten Jun 8, 2021

codeboten Jun 8, 2021

srikanthccv Jun 8, 2021

codeboten Jun 8, 2021

srikanthccv commented Jun 8, 2021

codeboten commented Jun 8, 2021

ocelotl Jun 8, 2021

srikanthccv Jun 8, 2021

ocelotl Jun 8, 2021

srikanthccv Jun 8, 2021 •

edited

srikanthccv commented Jun 8, 2021

codeboten commented Jun 8, 2021

srikanthccv commented Jun 9, 2021 •

edited

codeboten left a comment

		from opentelemetry.sdk.logs import LogData


		class LogExportResult(enum.Enum):

Add initial overall structure and classes for logs sdk #1894

Add initial overall structure and classes for logs sdk #1894

Conversation

srikanthccv commented Jun 5, 2021

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzchen commented Jun 8, 2021

codeboten left a 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

srikanthccv commented Jun 8, 2021

codeboten commented Jun 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srikanthccv Jun 8, 2021 • edited

Choose a reason for hiding this comment

srikanthccv commented Jun 8, 2021

codeboten commented Jun 8, 2021

srikanthccv commented Jun 9, 2021 • edited

codeboten left a comment

Choose a reason for hiding this comment

srikanthccv Jun 8, 2021 •

edited

srikanthccv commented Jun 9, 2021 •

edited