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

Update shared and remove statsd usage #803

Merged
merged 2 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 0 additions & 7 deletions helpers/checkpoint_logger/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
)

import sentry_sdk
from shared.metrics import metrics

from helpers.checkpoint_logger.prometheus import PROMETHEUS_HANDLER

Expand All @@ -31,7 +30,6 @@ def _error(msg, flow, strict=False):
# When a new version of worker rolls out, it will pick up tasks that
# may have been enqueued by the old worker and be missing checkpoints
# data. At least for that reason, we want to allow failing softly.
metrics.incr("worker.checkpoint_logger.error")
PROMETHEUS_HANDLER.log_errors(flow=flow.__name__)
if strict:
raise ValueError(msg)
Expand Down Expand Up @@ -273,12 +271,10 @@ class MyEnum(str, Enum):
"""

def log_counters(obj: T) -> None:
metrics.incr(f"{klass.__name__}.events.{obj.name}")
PROMETHEUS_HANDLER.log_checkpoints(flow=klass.__name__, checkpoint=obj.name)

# If this is the first checkpoint, increment the number of flows we've begun
if obj == next(iter(klass.__members__.values())):
metrics.incr(f"{klass.__name__}.total.begun")
PROMETHEUS_HANDLER.log_begun(flow=klass.__name__)
return

Expand All @@ -287,14 +283,11 @@ def log_counters(obj: T) -> None:
is_terminal = is_failure or is_success

if is_failure:
metrics.incr(f"{klass.__name__}.total.failed")
PROMETHEUS_HANDLER.log_failure(flow=klass.__name__)
elif is_success:
metrics.incr(f"{klass.__name__}.total.succeeded")
PROMETHEUS_HANDLER.log_success(flow=klass.__name__)

if is_terminal:
metrics.incr(f"{klass.__name__}.total.ended")
PROMETHEUS_HANDLER.log_total_ended(flow=klass.__name__)

klass.log_counters = log_counters
Expand Down
29 changes: 0 additions & 29 deletions helpers/tests/unit/test_checkpoint_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import pytest
from prometheus_client import REGISTRY
from shared.utils.test_utils import mock_metrics

from helpers.checkpoint_logger import (
BaseFlow,
Expand Down Expand Up @@ -354,9 +353,6 @@ def test_subflow_autosubmit(self, mock_sentry, mock_timestamp):
)

def test_reliability_counters(self):
metrics = mock_metrics(self.mocker)
assert not metrics.data

checkpoints = CheckpointLogger(DecoratedEnum)

counter_assertions = [
Expand All @@ -380,11 +376,6 @@ def test_reliability_counters(self):
]
with CounterAssertionSet(counter_assertions):
checkpoints.log(DecoratedEnum.BEGIN)
assert metrics.data["DecoratedEnum.events.BEGIN"] == 1
assert metrics.data["DecoratedEnum.total.begun"] == 1
assert "DecoratedEnum.total.succeeded" not in metrics.data
assert "DecoratedEnum.total.failed" not in metrics.data
assert "DecoratedEnum.total.ended" not in metrics.data

# Nothing special about `CHECKPOINT` - no counters should change
counter_assertions = [
Expand Down Expand Up @@ -413,11 +404,6 @@ def test_reliability_counters(self):
]
with CounterAssertionSet(counter_assertions):
checkpoints.log(DecoratedEnum.CHECKPOINT)
assert metrics.data["DecoratedEnum.events.CHECKPOINT"] == 1
assert metrics.data["DecoratedEnum.total.begun"] == 1
assert "DecoratedEnum.total.succeeded" not in metrics.data
assert "DecoratedEnum.total.failed" not in metrics.data
assert "DecoratedEnum.total.ended" not in metrics.data

# Failures should increment both `failed` and `ended`
counter_assertions = [
Expand All @@ -441,11 +427,6 @@ def test_reliability_counters(self):
]
with CounterAssertionSet(counter_assertions):
checkpoints.log(DecoratedEnum.BRANCH_1_FAIL)
assert metrics.data["DecoratedEnum.events.BRANCH_1_FAIL"] == 1
assert metrics.data["DecoratedEnum.total.begun"] == 1
assert metrics.data["DecoratedEnum.total.failed"] == 1
assert metrics.data["DecoratedEnum.total.ended"] == 1
assert "DecoratedEnum.total.succeeded" not in metrics.data

# Successes should increment both `succeeded` and `ended`
counter_assertions = [
Expand All @@ -469,11 +450,6 @@ def test_reliability_counters(self):
]
with CounterAssertionSet(counter_assertions):
checkpoints.log(DecoratedEnum.BRANCH_1_SUCCESS)
assert metrics.data["DecoratedEnum.events.BRANCH_1_SUCCESS"] == 1
assert metrics.data["DecoratedEnum.total.begun"] == 1
assert metrics.data["DecoratedEnum.total.failed"] == 1
assert metrics.data["DecoratedEnum.total.ended"] == 2
assert metrics.data["DecoratedEnum.total.succeeded"] == 1

# A different success path should also increment `succeeded` and `ended`
counter_assertions = [
Expand All @@ -497,11 +473,6 @@ def test_reliability_counters(self):
]
with CounterAssertionSet(counter_assertions):
checkpoints.log(DecoratedEnum.BRANCH_2_SUCCESS)
assert metrics.data["DecoratedEnum.events.BRANCH_2_SUCCESS"] == 1
assert metrics.data["DecoratedEnum.total.begun"] == 1
assert metrics.data["DecoratedEnum.total.failed"] == 1
assert metrics.data["DecoratedEnum.total.ended"] == 3
assert metrics.data["DecoratedEnum.total.succeeded"] == 2

def test_serialize_between_tasks(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem
https://github.com/codecov/shared/archive/795495e3d290c2f946792ee1a35f45fba5913c1b.tar.gz#egg=shared
https://github.com/codecov/shared/archive/4b937001e7c3ac9ad22ec7c9b0e83ac4135566fe.tar.gz#egg=shared
https://github.com/codecov/test-results-parser/archive/ef39a0888acd62d02a316a852a15d755c74e78c6.tar.gz#egg=test-results-parser
https://github.com/codecov/timestring/archive/d37ceacc5954dff3b5bd2f887936a98a668dda42.tar.gz#egg=timestring
asgiref>=3.7.2
Expand Down
10 changes: 5 additions & 5 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ opentelemetry-semantic-conventions==0.45b0
# opentelemetry-instrumentation-celery
# opentelemetry-sdk
orjson==3.10.7
# via -r requirements.in
# via
# -r requirements.in
# shared
packaging==24.1
# via pytest
platformdirs==3.11.0
Expand Down Expand Up @@ -359,7 +361,7 @@ sentry-sdk[celery]==2.13.0
# via
# -r requirements.in
# shared
shared @ https://github.com/codecov/shared/archive/795495e3d290c2f946792ee1a35f45fba5913c1b.tar.gz
shared @ https://github.com/codecov/shared/archive/4b937001e7c3ac9ad22ec7c9b0e83ac4135566fe.tar.gz
# via -r requirements.in
six==1.16.0
# via
Expand Down Expand Up @@ -388,9 +390,7 @@ sqlalchemy-utils==0.36.8
sqlparse==0.5.0
# via django
statsd==3.3.0
# via
# -r requirements.in
# shared
# via -r requirements.in
stripe==9.6.0
# via -r requirements.in
test-results-parser @ https://github.com/codecov/test-results-parser/archive/ef39a0888acd62d02a316a852a15d755c74e78c6.tar.gz
Expand Down
574 changes: 154 additions & 420 deletions services/report/languages/tests/unit/node/node1-result.json

Large diffs are not rendered by default.

Loading
Loading