8000 Merge branch 'master' into fix-aws-lambda-integration · davidcroda/sentry-python@cf09720 · GitHub
[go: up one dir, main page]
Skip to content < 8000 react-partial partial-name="keyboard-shortcuts-dialog" data-ssr="false" data-attempted-ssr="false" data-react-profiling="false" >

Commit cf09720

Browse files
sentrivanasentrivana
authored
Merge branch 'master' into fix-aws-lambda-integration
2 parents 2792d17 + f9ffe96 commit cf09720

File tree

5 files changed

+123
-36
lines changed

5 files changed

+123
-36
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## 1.39.0
4+
5+
### Various fixes & improvements
6+
7+
- Improve location reporting for timer metrics (#2552) by @mitsuhiko
8+
39
## 1.38.0
410

511
### Various fixes & improvements

‎sentry_sdk/metrics.py

Lines changed: 69 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
def get_code_location(stacklevel):
7272
# type: (int) -> Optional[Dict[str, Any]]
7373
try:
74-
frm = sys._getframe(stacklevel + 4)
74+
frm = sys._getframe(stacklevel)
7575
except Exception:
7676
return None
7777

@@ -508,7 +508,7 @@ def add(
508508
tags, # type: Optional[MetricTags]
509509
timestamp=None, # type: Optional[Union[float, datetime]]
510510
local_aggregator=None, # type: Optional[LocalAggregator]
511-
stacklevel=0, # type: int
511+
stacklevel=0, # type: Optional[int]
512512
):
513513
# type: (...) -> None
514514
if not self._ensure_thread() or self._flusher is None:
@@ -541,25 +541,9 @@ def add(
541541
previous_weight = 0
542542

543543
added = metric.weight - previous_weight
544-
self._buckets_total_weight += added
545544

546-
# Store code location once per metric and per day (of bucket timestamp)
547-
if self._enable_code_locations:
548-
meta_key = (ty, key, unit)
549-
start_of_day = utc_from_timestamp(timestamp).replace(
550-
hour=0, minute=0, second=0, microsecond=0, tzinfo=None
551-
)
552-
start_of_day = int(to_timestamp(start_of_day))
553-
554-
if (start_of_day, meta_key) not in self._seen_locations:
555-
self._seen_locations.add((start_of_day, meta_key))
556-
loc = get_code_location(stacklevel)
557-
if loc is not None:
558-
# Group metadata by day to make flushing more efficient.
559-
# There needs to be one envelope item per timestamp.
560-
self._pending_locations.setdefault(start_of_day, []).append(
561-
(meta_key, loc)
562-
)
545+
if stacklevel is not None:
546+
self.record_code_location(ty, key, unit, stacklevel + 2, timestamp)
563547

564548
# Given the new weight we consider whether we want to force flush.
565549
self._consider_force_flush()
@@ -568,6 +552,53 @@ def add(
568552
local_value = float(added if ty == "s" else value)
569553
local_aggregator.add(ty, key, local_value, unit, serialized_tags)
570554

555+
def record_code_location(
556+
self,
557+
ty, # type: MetricType
558+
key, # type: str
559+
unit, # type: MeasurementUnit
560+
stacklevel, # type: int
561+
timestamp=None, # type: Optional[float]
562+
):
563+
# type: (...) -> None
564+
if not self._enable_code_locations:
565+
return
566+
if timestamp is None:
567+
timestamp = time.time()
568+
meta_key = (ty, key, unit)
569+
start_of_day = utc_from_timestamp(timestamp).replace(
570+
hour=0, minute=0, second=0, microsecond=0, tzinfo=None
571+
)
572+
start_of_day = int(to_timestamp(start_of_day))
573+
574+
if (start_of_day, meta_key) not in self._seen_locations:
575+
self._seen_locations.add((start_of_day, meta_key))
576+
loc = get_code_location(stacklevel + 3)
577+
if loc is not None:
578+
# Group metadata by day to make flushing more efficient.
579+
# There needs to be one envelope item per timestamp.
580+
self._pending_locations.setdefault(start_of_day, []).append(
581+
(meta_key, loc)
582+
)
583+
584+
@metrics_noop
585+
def need_code_loation(
586+
self,
587+
ty, # type: MetricType
588+
key, # type: str
589+
unit, # type: MeasurementUnit
590+
timestamp, # type: float
591+
):
592+
# type: (...) -> bool
593+
if self._enable_code_locations:
594+
return False
595+
meta_key = (ty, key, unit)
596+
start_of_day = utc_from_timestamp(timestamp).replace(
597+
hour=0, minute=0, second=0, microsecond=0, tzinfo=None
598+
)
599+
start_of_day = int(to_timestamp(start_of_day))
600+
return (start_of_day, meta_key) not in self._seen_locations
601+
571602
def kill(self):
572603
# type: (...) -> None
573604
if self._flusher is None:
@@ -651,9 +682,19 @@ def _tags_to_dict(tags):
651682
return rv
652683

653684

685+
def _get_aggregator():
686+
# type: () -> Optional[MetricsAggregator]
687+
hub = sentry_sdk.Hub.current
688+
client = hub.client
689+
return (
690+
client.metrics_aggregator
691+
if client is not None and client.metrics_aggregator is not None
692+
else None
693+
)
694+
695+
654696
def _get_aggregator_and_update_tags(key, tags):
655697
# type: (str, Optional[MetricTags]) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]]
656-
"""Returns the current metrics aggregator if there is one."""
657698
hub = sentry_sdk.Hub.current
658699
client = hub.client
659700
if client is None or client.metrics_aggregator is None:
@@ -751,6 +792,12 @@ def __enter__(self):
751792
value = ",".join(sorted(map(str, value)))
752793
self._span.set_tag(key, value)
753794
self._span.__enter__()
795+
796+
# report code locations here for better accuracy
797+
aggregator = _get_aggregator()
798+
if aggregator is not None:
799+
aggregator.record_code_location("d", self.key, self.unit, self.stacklevel)
800+
754801
return self
755802

756803
def __exit__(self, exc_type, exc_value, tb):
@@ -769,7 +816,7 @@ def __exit__(self, exc_type, exc_value, tb):
769816
tags,
770817
self.timestamp,
771818
local_aggregator,
772-
self.stacklevel,
819+
None, # code locations are reported in __enter__
773820
)
774821

775822
self._span.__exit__(exc_type, exc_value, tb)

‎sentry_sdk/utils.py

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,16 +95,11 @@ def _get_debug_hub():
9595
pass
9696

9797

98-
def get_default_release():
98+
def get_git_revision():
9999
# type: () -> Optional[str]
100-
"""Try to guess a default release."""
101-
release = os.environ.get("SENTRY_RELEASE")
102-
if release:
103-
return release
104-
105100
with open(os.path.devnull, "w+") as null:
106101
try:
107-
release = (
102+
revision = (
108103
subprocess.Popen(
109104
["git", "rev-parse", "HEAD"],
110105
stdout=subprocess.PIPE,
@@ -116,10 +111,21 @@ def get_default_release():
116111
.decode("utf-8")
117112
)
118113
except (OSError, IOError):
119-
pass
114+
return None
120115

121-
if release:
122-
return release
116+
return revision
117+
118+
119+
def get_default_release():
120+
# type: () -> Optional[str]
121+
"""Try to guess a default release."""
122+
release = os.environ.get("SENTRY_RELEASE")
123+
if release:
124+
return release
125+
126+
release = get_git_revision()
127+
if release is not None:
128+
return release
123129

124130
for var in (
125131
"HEROKU_SLUG_COMMIT",

‎tests/integrations/aws_lambda/client.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import tempfile
99

1010
from sentry_sdk.consts import VERSION as SDK_VERSION
11+
from sentry_sdk.utils import get_git_revision
1112

1213
AWS_REGION_NAME = "us-east-1"
1314
AWS_CREDENTIALS = {
@@ -226,7 +227,8 @@ def run_lambda_function(
226227
# Making a unique function name depending on all the code that is run in it (function code plus SDK version)
227228
# The name needs to be short so the generated event/envelope json blobs are small enough to be output
228229
# in the log result of the Lambda function.
229-
function_hash = hashlib.shake_256((code + SDK_VERSION).encode("utf-8")).hexdigest(5)
230+
rev = get_git_revision() or SDK_VERSION
231+
function_hash = hashlib.shake_256((code + rev).encode("utf-8")).hexdigest(6)
230232
fn_name = "test_{}".format(function_hash)
231233
full_fn_name = "{}_{}".format(
232234
fn_name, runtime.replace(".", "").replace("python", "py")

‎tests/test_metrics.py

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import sys
44
import time
5+
import linecache
56

67
from sentry_sdk import Hub, metrics, push_scope, start_transaction
78
from sentry_sdk.tracing import TRANSACTION_SOURCE_ROUTE
@@ -126,7 +127,8 @@ def test_timing(sentry_init, capture_envelopes):
126127
}
127128

128129
assert meta_item.headers["type"] == "metric_meta"
129-
assert parse_json(meta_item.payload.get_bytes()) == {
130+
json = parse_json(meta_item.payload.get_bytes())
131+
assert json == {
130132
"timestamp": mock.ANY,
131133
"mapping": {
132134
"d:whatever@second": [
@@ -145,6 +147,13 @@ def test_timing(sentry_init, capture_envelopes):
145147
},
146148
}
147149

150+
loc = json["mapping"]["d:whatever@second"][0]
151+
line = linecache.getline(loc["abs_path"], loc["lineno"])
152+
assert (
153+
line.strip()
154+
== 'with metrics.timing("whatever", tags={"blub": "blah"}, timestamp=ts):'
155+
)
156+
148157

149158
def test_timing_decorator(sentry_init, capture_envelopes):
150159
sentry_init(
@@ -196,7 +205,8 @@ def amazing_nano():
196205
}
197206

198207
assert meta_item.headers["type"] == "metric_meta"
199-
assert parse_json(meta_item.payload.get_bytes()) == {
208+
json = parse_json(meta_item.payload.get_bytes())
209+
assert json == {
200210
"timestamp": mock.ANY,
201211
"mapping": {
202212
"d:whatever-1@second": [
@@ -228,6 +238,14 @@ def amazing_nano():
228238
},
229239
}
230240

241+
# XXX: this is not the best location. It would probably be better to
242+
# report the location in the function, however that is quite a bit
243+
# tricker to do since we report from outside the function so we really
244+
# only see the callsite.
245+
loc = json["mapping"]["d:whatever-1@second"][0]
246+
line = linecache.getline(loc["abs_path"], loc["lineno"])
247+
assert line.strip() == "assert amazing() == 42"
248+
231249

232250
def test_timing_basic(sentry_init, capture_envelopes):
233251
sentry_init(
@@ -316,7 +334,8 @@ def test_distribution(sentry_init, capture_envelopes):
316334
}
317335

318336
assert meta_item.headers["type"] == "metric_meta"
319-
assert parse_json(meta_item.payload.get_bytes()) == {
337+
json = parse_json(meta_item.payload.get_bytes())
338+
assert json == {
320339
"timestamp": mock.ANY,
321340
"mapping": {
322341
"d:dist@none": [
@@ -335,6 +354,13 @@ def test_distribution(sentry_init, capture_envelopes):
335354
},
336355
}
337356

357+
loc = json["mapping"]["d:dist@none"][0]
695F
358+
line = linecache.getline(loc["abs_path"], loc["lineno"])
359+
assert (
360+
line.strip()
361+
== 'metrics.distribution("dist", 1.0, tags={"a": "b"}, timestamp=ts)'
362+
)
363+
338364

339365
def test_set(sentry_init, capture_envelopes):
340366
sentry_init(

0 commit comments

Comments
 (0)
0