Skip to content

Obinna/s3 en 2674 disable logging #289

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 20, 2024
Merged

Obinna/s3 en 2674 disable logging #289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67b6bab
disable print statements if disable_logging is set to true
obinnascale3 Aug 19, 2024
882ec7e
update version
obinnascale3 Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions src/langtrace_python_sdk/extensions/langtrace_exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import os
import typing
import sys

import requests
from opentelemetry.sdk.trace.export import ReadableSpan, SpanExporter, SpanExportResult
Expand Down Expand Up @@ -49,18 +50,21 @@ class LangTraceExporter(SpanExporter):

api_key: str
api_host: str
disable_logging: bool

def __init__(
self,
api_host,
api_key: str = None,
disable_logging: bool = False,
) -> None:
self.api_key = api_key or os.environ.get("LANGTRACE_API_KEY")
self.api_host = (
f"{LANGTRACE_REMOTE_URL}/api/trace"
if api_host == LANGTRACE_REMOTE_URL
else api_host
)
self.disable_logging = disable_logging

def export(self, spans: typing.Sequence[ReadableSpan]) -> SpanExportResult:
"""
Expand All @@ -72,7 +76,7 @@ def export(self, spans: typing.Sequence[ReadableSpan]) -> SpanExportResult:
Returns:
The result of the export SUCCESS or FAILURE
"""
if not self.api_key:
if not self.api_key and not self.disable_logging:
Copy link
Collaborator

@alizenhom alizenhom Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am thinking a bit about this,

this approach will work fine for now, but it's adding some tech debt to the langtrace and langtrace exporter
i would say a more scalable approach is the following

  1. make the disable logging as an enviroment variable not an option to be passed
  2. abstract all Prints with Fore into a seperate function inside utils, that recieves 2 arguments which are the message and the color
  3. inside the abstracted method just check for the env variable if it's set then return and do not print other wise print normally
Suggested change
if not self.api_key and not self.disable_logging:
def log(message, color):
disable_logging = os.environ['DISABLE_LOGGING']
if disable_logging:
return
print(color, message, FORE.reset)

what do you think?

print(Fore.RED)
print(
"Missing Langtrace API key, proceed to https://langtrace.ai to create one"
Expand Down Expand Up @@ -107,14 +111,15 @@ def export(self, spans: typing.Sequence[ReadableSpan]) -> SpanExportResult:

if not response.ok:
raise RequestException(response.text)

print(
Fore.GREEN + f"Exported {len(spans)} spans successfully." + Fore.RESET
)
if not self.disable_logging:
print(
Fore.GREEN + f"Exported {len(spans)} spans successfully." + Fore.RESET
)
return SpanExportResult.SUCCESS
except RequestException as err:
print(Fore.RED + "Failed to export spans.")
print(Fore.RED + f"Error: {err}" + Fore.RESET)
if not self.disable_logging:
print(Fore.RED + "Failed to export spans.")
print(Fore.RED + f"Error: {err}" + Fore.RESET)
return SpanExportResult.FAILURE

def shutdown(self) -> None:
Expand Down
7 changes: 6 additions & 1 deletion src/langtrace_python_sdk/langtrace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ def init(
disable_instrumentations: Optional[DisableInstrumentations] = None,
disable_tracing_for_functions: Optional[InstrumentationMethods] = None,
service_name: Optional[str] = None,
disable_logging = False
):
if disable_logging:
sys.stdout = open(os.devnull, "w")
Copy link
Collaborator

@alizenhom alizenhom Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain what does this do?


host = (
os.environ.get("LANGTRACE_API_HOST", None) or api_host or LANGTRACE_REMOTE_URL
Expand All @@ -90,7 +93,7 @@ def init(
provider = TracerProvider(resource=resource, sampler=sampler)

remote_write_exporter = (
LangTraceExporter(api_key=api_key, api_host=host)
LangTraceExporter(api_key=api_key, api_host=host, disable_logging=disable_logging)
if custom_remote_exporter is None
else custom_remote_exporter
)
Expand Down Expand Up @@ -146,6 +149,8 @@ def init(
print(Fore.BLUE + "Exporting spans to Langtrace cloud.." + Fore.RESET)
provider.add_span_processor(batch_processor_remote)

sys.stdout = sys.__stdout__
Copy link
Collaborator

@alizenhom alizenhom Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain what does this do?



def init_instrumentations(
disable_instrumentations: DisableInstrumentations, all_instrumentations: dict
Expand Down
2 changes: 1 addition & 1 deletion src/langtrace_python_sdk/version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "2.2.21"
__version__ = "2.2.22"