Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom output function in console exporter #411

Closed
AntonKueltz opened this issue Feb 12, 2020 · 5 comments · Fixed by #412
Closed

Custom output function in console exporter #411

AntonKueltz opened this issue Feb 12, 2020 · 5 comments · Fixed by #412

Comments

@AntonKueltz
Copy link

AntonKueltz commented Feb 12, 2020

What do folks think of making the output function used by ConsoleSpanExporter.export customizable? Currently it uses print(span) but there are situations where a user may want to perform a transform on span before outputting it, or to output another data structure entirely.

To be more concrete, we currently have downstream infrastructure that requires our telemetry data to be in a specific dictionary format when written to stdout. We'd like to still use the ConsoleSpanExporter to write to stdout, but need to convert the span to our dict object before doing so.

Currently we are addressing this by writing a custom exporter class that inherits from SpanExporter and writes our custom data to stdout in its export method. Another solution though would be to allow ConsoleSpanExporter to take a keyword argument to its __init__ method that would default to print but would also allow custom output functions. It would then use the passed function in ConsoleSpanExporter.export to output to stdout.

Do folks see any value in adding this functionality here, rather than requiring users who want to do custom console output to write their own exporter classes? Also happy to write up the proposed solution as a PR if folks think that would be helpful.

@ocelotl
Copy link
Contributor

ocelotl commented Feb 12, 2020

I see a lot of value in this! In fact, some time ago I had modified the exporter in place to print the output in a dictionary/JSON-like output where the parent-child relationship of spans could be displayed as they were nested.

I think it is better to leave ConsoleSpanExporter as it is now and instead do as you do by inheriting from SpanExporter. Even better, we can define an ABC for exporters, then use entry points to allow new exporters to plug in, etc.

Sure, open a PR with your code, please 👍

@c24t
Copy link
Member

c24t commented Feb 12, 2020

Hey @AntonKueltz, welcome to the project.

I agree wth making SpanExporter an interface, and in general I think this is the right approach for custom exporters. But in this particular case I think there's a good argument that this could have been built into ConsoleSpanExporter from the beginning. Something like:

class ConsoleSpanExporter(SpanExporter):
    """Implementation of :class:`SpanExporter` that prints spans to the
    console.

    This class can be used for diagnostic purposes. It prints the exported
    spans to the console STDOUT.
    """

    def __init__(
        self,
        out: typing.IO = sys.stdout,
        munge: typing.Callable[[Span], str] = str,
    ):
        self.out = out
        self.munge = format

    def export(self, spans: typing.Sequence[Span]) -> SpanExportResult:
        for span in spans:
            self.out.write(self.munge(span))
        return SpanExportResult.SUCCESS

It's a pretty short diff, and doesn't affect the default behavior of the exporter:

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py
index aa8efac3..d64d9323 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py
@@ -14,6 +14,7 @@

 import collections
 import logging
+import sys
 import threading
 import typing
 from enum import Enum
@@ -268,7 +269,15 @@ class ConsoleSpanExporter(SpanExporter):
     spans to the console STDOUT.
     """

+    def __init__(
+        self,
+        out: typing.IO = sys.stdout,
+        munge: typing.Callable[[Span], str] = str,
+    ):
+        self.out = out
+        self.munge = format
+
     def export(self, spans: typing.Sequence[Span]) -> SpanExportResult:
         for span in spans:
-            print(span)
+            self.out.write(self.munge(span))
         return SpanExportResult.SUCCESS

c24t added a commit to c24t/opentelemetry-python that referenced this issue Feb 12, 2020
c24t added a commit to c24t/opentelemetry-python that referenced this issue Feb 12, 2020
@AntonKueltz
Copy link
Author

AntonKueltz commented Feb 12, 2020

Thanks for the feedback @ocelotl and @c24t. The code @c24t provided is pretty much what I had in mind (with the nice addition of also being able to define the output stream).

Regarding the SpanExporter, it currently works well enough for using as a base class and plugging the custom child class into a span processor. But there may be more sophisticated use cases than our team currently has that can be considered (as @ocelotl alluded to already).

I'll go ahead and PR the changes I have (to include the custom output stream as well) unless anyone has any further comments or concerns?

Edit - @c24t beat me to the PR haha

@c24t
Copy link
Member

c24t commented Feb 12, 2020

Edit - @c24t beat me to the PR haha

Ah sorry, it was a quick enough change that I thought I might as well put it in a PR. If this isn't what you need feel free to open another one to replace it, or feel free to push to that branch.

@ocelotl LGTY?

@AntonKueltz
Copy link
Author

Nope, this looks good to me, thanks for the quick turnaround!

@c24t c24t closed this as completed in #412 Feb 13, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants