-
Notifications
You must be signed in to change notification settings - Fork 1
Add distributed tracing module for New Relic integration #180
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
d8a5cd3
163482a
6826464
ceff470
2002fbd
2ebb1ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| from .tracing import ( | ||
| TRACE_HEADERS_KEY, | ||
| accept_trace_from_queue, | ||
| accept_trace_headers, | ||
| add_custom_attribute, | ||
| background_task, | ||
| get_trace_headers, | ||
| inject_trace_headers, | ||
| trace_attributes, | ||
| ) | ||
|
|
||
| __all__ = [ | ||
| 'TRACE_HEADERS_KEY', | ||
| 'accept_trace_from_queue', | ||
| 'accept_trace_headers', | ||
| 'add_custom_attribute', | ||
| 'background_task', | ||
| 'get_trace_headers', | ||
| 'inject_trace_headers', | ||
| 'trace_attributes', | ||
| ] | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,223 @@ | ||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||
|
|
||||||||||||||||||
| import inspect | ||||||||||||||||||
| from contextlib import contextmanager | ||||||||||||||||||
| from functools import wraps | ||||||||||||||||||
| from typing import Any, Callable, Optional | ||||||||||||||||||
|
|
||||||||||||||||||
| import newrelic.agent | ||||||||||||||||||
|
||||||||||||||||||
| try: | |
| from aiobotocore.session import get_session | |
| from types_aiobotocore_sqs import SQSClient | |
| except ImportError: | |
| raise ImportError( | |
| "You must install agave with [asyncio_aws_tools] option.\n" | |
| "You can install it with: pip install agave[asyncio_aws_tools]" | |
| ) |
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.
listo
dpastranak marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
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.
Aquí podemos ser más específicos dict[str, str]
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.
lsito
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.
Esta funcion es un poco extraña, inserta una lista vacía y el resultado siempre es el mismo.
Creo que es mejor quitar la funcion y hacer directo el insert_distributed_trace_headers
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.
es que más bien el API de newrelic está extraña. Pide pasar una lista vacía que muta internamente donde regresa los headers y esta función encapsula esta complejidad.
sin esta función se tendría que hacer
headers_list = []
newrelic.agent.insert_distributed_trace_headers(headers_list)
# Después de llamar, headers_list ya tiene datos:
# [('newrelic', 'eyJ2IjpbMCwxXS...'), ('traceparent', '00-abc123...')]
headers = dict(headers_list) # lo paso a un diccionario para pasarlo entre servicio.
con la función solo se hace
headers = get_trace_headers()
o por qué dices que siempre regresa lo mismo?
Creo que justo esta función hace la interfaz más clara y simple.
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.
Esta función no es necesaria.
Realmente solo hace una linea y el transport_type lo usas como Queue
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.
En este caso la función también incluye la validación if not headers: return que evita errores cuando los headers son None. Además, el transport_type por default es "HTTP" y se puede usar en contexto HHTP no solo como Queue.
Sin esta función cada que se use tendría que repetirse la validación también. Creo que la función aporta valor.
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| __version__ = '1.5.2' | ||
| __version__ = "1.5.3.dev01" | ||
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,3 +12,4 @@ moto[server]==5.0.26 | |
| pytest-vcr==1.0.2 | ||
| pytest-asyncio==0.18.* | ||
| typing_extensions==4.12.2 | ||
| newrelic==11.2.0 | ||
|
||
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.
Sugiero quitarlos de aqui.
Para el modulo
agave.coreno dependa de newrelicThere 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.
lsito