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

Open Telemetry #819

Merged
merged 45 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
3906dd1
feat: adhoc: Update pre-commit hooks and Dockerfile for local dev
Xaelias Dec 22, 2023
c89b58e
feat: adhoc: Install opentelemetry pacakges, create Reddit trace prop…
Xaelias Dec 22, 2023
8875362
feat: adhoc: Implement opentelemetry for thrift client and server
Xaelias Dec 22, 2023
ca26600
feat: adhoc: Instrument pyramid/request with Opentelemetry
Xaelias Dec 22, 2023
3603ce0
trying to move the auto instrumentation around as requested
Xaelias Jan 22, 2024
82405bc
Addressed some PR comments
Mar 20, 2024
3ccd636
Change these back from BaseException to Exception
Mar 20, 2024
46459d9
Switch thrift back to and record the exception to otel.
Mar 20, 2024
1213039
Don't explicitly record the exception as we enabled the auto exceptio…
Mar 20, 2024
2382d2e
Update propagator to correctly follow baseplate spec using base10 int…
Mar 20, 2024
448ae17
Apply linting fixes
Mar 21, 2024
ee175eb
fix
Mar 21, 2024
40f8688
Fix pyramid tests to reflect the baseplate spec's use of base10 trace…
Mar 21, 2024
fefd6d2
Update how we configure propagators so that we support the different …
Mar 21, 2024
16202e6
Fix formatting and linting
Mar 21, 2024
2410d6b
regex fix
trevorriles Apr 4, 2024
3f56f61
regex fix x2
trevorriles Apr 4, 2024
3d7ef32
Simplify conditional logic in server init
trevorriles Apr 4, 2024
0825a6e
Move instrumentation back to modules instead of __init__ (#846)
trevorriles Apr 11, 2024
de5f9ee
Trevorriles/temp (#910)
trevorriles May 6, 2024
42fcdf0
Rebase fixes
May 6, 2024
3905041
Update cached_property wrapper
May 6, 2024
a5c750a
Add back in baseplate thrift errorcode fix
May 6, 2024
f194423
Fix indentation error made during rebase
May 16, 2024
f35abeb
Set doc pyproject black language level to match top-level project
May 16, 2024
4ca3656
Fix mypy type that was missed during rebase
May 16, 2024
62c8347
Remove reset_metrics method in tests that did not call it
May 16, 2024
5216cfe
Move baseplate.py sampling and configuration to be more in line with …
trevorriles May 31, 2024
7969d5a
Merge branch 'develop' of github.com:reddit/baseplate.py into aleksei…
Jun 1, 2024
9834885
make fmt
Jun 1, 2024
e478766
Update baseplate/frameworks/thrift/__init__.py
trevorriles Jun 6, 2024
90206a1
Bump lxml from 5.2.1 to 5.2.2 (#921)
dependabot[bot] Jun 3, 2024
ca45736
Bump boto3 from 1.34.95 to 1.34.117 (#922)
dependabot[bot] Jun 3, 2024
010118e
Bump actions/checkout from 4.1.4 to 4.1.6 (#927)
dependabot[bot] Jun 3, 2024
4b9af5d
v2.7.0
chriskuehl Jun 4, 2024
6ab0c0c
Bump urllib3 from 1.26.18 to 1.26.19 (#931)
dependabot[bot] Jun 18, 2024
fbe77fd
Bump typing-extensions from 4.12.0 to 4.12.2 (#939)
dependabot[bot] Jul 1, 2024
5dd625a
Bump moto from 5.0.9 to 5.0.10 (#938)
dependabot[bot] Jul 1, 2024
853fe62
Bump pylint from 3.2.2 to 3.2.5 (#937)
dependabot[bot] Jul 1, 2024
e03785d
Bump flake8 from 7.0.0 to 7.1.0 (#936)
dependabot[bot] Jul 1, 2024
3a5858a
Bump types-setuptools from 70.0.0.20240524 to 70.1.0.20240627 (#935)
dependabot[bot] Jul 1, 2024
f2301a9
Bump boto3 from 1.34.117 to 1.34.136 (#932)
dependabot[bot] Jul 1, 2024
1aa4252
Bump certifi from 2024.2.2 to 2024.7.4 (#944)
dependabot[bot] Jul 8, 2024
b925e67
Remove broken environment variable based configuration. Sampling conf…
Jul 23, 2024
cf426c7
Remove unused imports
Jul 23, 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
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@ repos:
name: make-fmt
stages: [commit]
language: system
entry: docker-compose run baseplate make fmt
entry: docker compose run --no-TTY baseplate make fmt
always_run: true
- id: make-lint
name: make-lint
stages: [push]
language: system
entry: docker-compose run baseplate make lint
entry: docker compose run --no-TTY baseplate make lint
always_run: true
fail_fast: true
- id: pytest
name: pytest
stages: [push]
language: system
entry: docker-compose run baseplate pytest
entry: docker compose run --no-TTY baseplate pytest
always_run: true
fail_fast: true
3 changes: 3 additions & 0 deletions baseplate/clients/requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from advocate import AddrValidator
from advocate import ValidatingHTTPAdapter
from opentelemetry.instrumentation.requests import RequestsInstrumentor
from prometheus_client import Counter
from prometheus_client import Gauge
from prometheus_client import Histogram
Expand All @@ -25,6 +26,8 @@
from baseplate.lib.prometheus_metrics import default_latency_buckets
from baseplate.lib.prometheus_metrics import getHTTPSuccessLabel

RequestsInstrumentor().instrument()
trevorriles marked this conversation as resolved.
Show resolved Hide resolved


def http_adapter_from_config(
app_config: config.RawConfig, prefix: str, **kwargs: Any
Expand Down
250 changes: 161 additions & 89 deletions baseplate/clients/thrift.py

Large diffs are not rendered by default.

32 changes: 23 additions & 9 deletions baseplate/frameworks/pyramid/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import pyramid.tweens
import webob.request

from opentelemetry import trace
from opentelemetry.instrumentation.pyramid import PyramidInstrumentor
from prometheus_client import Counter
from prometheus_client import Gauge
from prometheus_client import Histogram
Expand All @@ -34,11 +36,12 @@
from baseplate.lib.prometheus_metrics import getHTTPSuccessLabel
from baseplate.thrift.ttypes import IsHealthyProbe


logger = logging.getLogger(__name__)

PyramidInstrumentor().instrument()
Copy link
Member

Choose a reason for hiding this comment

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

Same question as before about instrumenting as an import-time side effect.



class SpanFinishingAppIterWrapper:
class SpanFinishingAppIterWrapper(Iterable):
"""Wrapper for Response.app_iter that finishes the span when the iterator is done.

The WSGI spec expects applications to return an iterable object. In the
Expand All @@ -53,7 +56,7 @@ class SpanFinishingAppIterWrapper:

"""

def __init__(self, span: Span, app_iter: Iterable[bytes]) -> None:
def __init__(self, app_iter: Iterator[bytes], span: Optional[Span] = None) -> None:
self.span = span
self.app_iter = iter(app_iter)

Expand All @@ -64,10 +67,15 @@ def __next__(self) -> bytes:
try:
return next(self.app_iter)
except StopIteration:
self.span.finish()
trace.get_current_span().set_status(trace.status.StatusCode.OK)
if self.span:
self.span.finish()
raise
except: # noqa: E722
self.span.finish(exc_info=sys.exc_info())
except Exception as e:
trace.get_current_span().set_status(trace.status.StatusCode.ERROR)
trace.get_current_span().record_exception(e)
if self.span:
self.span.finish(exc_info=sys.exc_info())
raise

def close(self) -> None:
Expand Down Expand Up @@ -129,15 +137,21 @@ def baseplate_tween(request: Request) -> Response:
response = handler(request)
if request.span:
request.span.set_tag("http.response_length", response.content_length)
except: # noqa: E722
except Exception as e:
trace.get_current_span().set_status(trace.status.StatusCode.ERROR)
trace.get_current_span().record_exception(e)
if hasattr(request, "span") and request.span:
request.span.finish(exc_info=sys.exc_info())
raise
else:
trace.get_current_span().set_status(trace.status.StatusCode.OK)
content_length = response.content_length
if request.span:
request.span.set_tag("http.status_code", response.status_code)
content_length = response.content_length
response.app_iter = SpanFinishingAppIterWrapper(request.span, response.app_iter)
response.app_iter = SpanFinishingAppIterWrapper(response.app_iter, request.span)
response.content_length = content_length
else:
response.app_iter = SpanFinishingAppIterWrapper(response.app_iter)
response.content_length = content_length
finally:
manually_close_request_metrics(request, response)
Expand Down
Loading
Loading