-
Notifications
You must be signed in to change notification settings - Fork 132
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
Add OpenTelemetry tracing #1355
Add OpenTelemetry tracing #1355
Conversation
44c7705
to
b443691
Compare
return wrapped_view | ||
except ImportError: | ||
def create_span(view): | ||
@functools.wraps(view) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
over-indented
|
||
return wrapped_view | ||
except ImportError: | ||
def create_span(view): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation is not a multiple of four
return view(**kwargs) | ||
|
||
return wrapped_view | ||
except ImportError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Black would make changes.
@wraps(f) | ||
def decorated(*args, **kwargs): | ||
return f(*args, **kwargs) | ||
return decorated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no newline at end of file
if args[0].full_name is not None: | ||
span.set_attribute("full_name", args[0].full_name) | ||
if args[0].authoritative_source is not None: | ||
span.set_attribute("authoritative_source", args[0].authoritative_source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (88 > 79 characters)
span.set_attribute("username", auth.username) | ||
if auth.full_name is not None: | ||
span.set_attribute("full_name", auth.full_name) | ||
span.set_attribute("authoritative_source", auth.authoritative_source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (89 > 79 characters)
for parameter in signature.parameters: | ||
if index > 1: | ||
try: | ||
span.set_attribute("argument." + parameter, str(args[index])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (89 > 79 characters)
|
||
auth = args[1] | ||
signature = inspect.signature(f) | ||
with tracer.start_as_current_span(args[0].__class__.__name__ + " " + f.__name__, context.get_current(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (115 > 79 characters)
return post(f'{endpoint}{path}', data=request.data, headers=request.headers).content | ||
|
||
|
||
def create_span(f): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too many blank lines (2)
def setup(app, endpoint): | ||
@app.route('/v1/traces/', defaults={'path': ''}, methods=["POST"]) | ||
def proxy(path): | ||
return post(f'{endpoint}{path}', data=request.data, headers=request.headers).content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (96 > 79 characters)
b443691
to
b7825dd
Compare
trace.set_tracer_provider(provider) | ||
|
||
|
||
def setup(app, endpoint): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too many blank lines (2)
tracer = trace.get_tracer("nipap") | ||
|
||
def init_tracing(service_name, endpoint): | ||
resource = Resource(attributes={ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Black would make changes.
|
||
from opentelemetry import trace, context | ||
from opentelemetry.trace import SpanKind, StatusCode | ||
from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (86 > 79 characters)
@@ -17,7 +17,7 @@ | |||
author_email = pynipap.__author_email__, | |||
license = pynipap.__license__, | |||
url = pynipap.__url__, | |||
py_modules = ['pynipap'], | |||
py_modules = ['pynipap','tracing'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing whitespace after ','
unexpected spaces around keyword / parameter equals
pynipap/tracing.py
Outdated
""" | ||
|
||
signature = inspect.signature(f) | ||
with tracer.start_as_current_span(args[0].__name__ + " " + f.__name__, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (82 > 79 characters)
if use_grpc: | ||
processor = BatchSpanProcessor(OTLPSpanExporter(endpoint=endpoint)) | ||
else: | ||
processor = BatchSpanProcessor(opentelemetry.exporter.otlp.proto.http.trace_exporter.OTLPSpanExporter(endpoint=endpoint)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (133 > 79 characters)
tracer = trace.get_tracer("pynipap") | ||
|
||
|
||
def init_tracing(service_name, endpoint, use_grpc=True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too many blank lines (2)
|
||
tracer = trace.get_tracer("pynipap") | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Black would make changes.
try: | ||
from opentelemetry import trace, context | ||
from opentelemetry.trace import SpanKind, INVALID_SPAN | ||
from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (86 > 79 characters)
if sys.version_info[0] < 3: | ||
import xmlrpclib | ||
|
||
int = long |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined name 'long'
ae0f3b1
to
9b1c79c
Compare
try: | ||
result = super().request(host, handler, request_body, verbose) | ||
current_span.set_attribute("http.status_code", 200) | ||
except xmlrpclib.ProtocolError as protocolError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we make sure to re-raise the exception, this'll silently drop all ProtocolErrors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we definitely should. I've pushed an update correcting this.
9b1c79c
to
20ae1f8
Compare
LGTM 👍 |
Add OpenTelemetry tracing for cli, www and nipapd