Merge branch 'master' into fix-no-error

getsentry · May 24, 2024 · 71ff7c5 · 71ff7c5
2 parents ace23a8 + 2ceeca4
commit 71ff7c5
Show file tree

Hide file tree

Showing 19 changed files with 485 additions and 112 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,23 @@
 # CHANGELOG
 
+## 4.7.0
+
+The Sentry SDK team is happy to announce the immediate availability of Sentry PHP SDK v4.7.0.
+
+### Features
+
+- Improve debugging experience by emitting more logs from the SDK [(#1705)](https://github.com/getsentry/sentry-php/pull/1705)
+- Handle `metric_bucket` rate limits [(#1726)](https://github.com/getsentry/sentry-php/pull/1726) & [(#1728)](https://github.com/getsentry/sentry-php/pull/1728)
+
+### Bug Fixes
+
+- Fix deprecation notice when trying to serialize a callable [(#1732)](https://github.com/getsentry/sentry-php/pull/1732)
+
+### Misc
+
+- Deprecated `SpanStatus::resourceExchausted()`. Use `SpanStatus::resourceExhausted()` instead [(#1725)](https://github.com/getsentry/sentry-php/pull/1725)
+- Update metric normalization [(#1729)](https://github.com/getsentry/sentry-php/pull/1729)
+
 ## 4.6.1
 
 The Sentry SDK team is happy to announce the immediate availability of Sentry PHP SDK v4.6.1.

diff --git a/src/Client.php b/src/Client.php
@@ -32,7 +32,7 @@ class Client implements ClientInterface
     /**
      * The version of the SDK.
      */
-    public const SDK_VERSION = '4.6.1';
+    public const SDK_VERSION = '4.7.0';
 
     /**
      * @var Options The client options

diff --git a/src/Metrics/Metrics.php b/src/Metrics/Metrics.php
@@ -9,6 +9,9 @@
 use Sentry\Metrics\Types\DistributionType;
 use Sentry\Metrics\Types\GaugeType;
 use Sentry\Metrics\Types\SetType;
+use Sentry\Tracing\SpanContext;
+
+use function Sentry\trace;
 
 class Metrics
 {
@@ -142,21 +145,32 @@ public function timing(
         array $tags = [],
         int $stackLevel = 0
     ) {
-        $startTimestamp = microtime(true);
-
-        $result = $callback();
-
-        $this->aggregator->add(
-            DistributionType::TYPE,
-            $key,
-            microtime(true) - $startTimestamp,
-            MetricsUnit::second(),
-            $tags,
-            (int) $startTimestamp,
-            $stackLevel
+        return trace(
+            function () use ($callback, $key, $tags, $stackLevel) {
+                $startTimestamp = microtime(true);
+
+                $result = $callback();
+
+                /**
+                 * Emitting the metric here, will attach it to the
+                 * "metric.timing" span.
+                 */
+                $this->aggregator->add(
+                    DistributionType::TYPE,
+                    $key,
+                    microtime(true) - $startTimestamp,
+                    MetricsUnit::second(),
+                    $tags,
+                    (int) $startTimestamp,
+                    $stackLevel + 4 // the `trace` helper adds 4 additional stack frames
+                );
+
+                return $result;
+            },
+            SpanContext::make()
+                ->setOp('metric.timing')
+                ->setDescription($key)
         );
-
-        return $result;
     }
 
     public function flush(): ?EventId

diff --git a/src/Options.php b/src/Options.php
@@ -130,6 +130,8 @@ public function getTracesSampleRate(): ?float
      * precedence.
      *
      * @param bool|null $enableTracing Boolean if tracing should be enabled or not
+     *
+     * @deprecated since version 4.7. To be removed in version 5.0
      */
     public function setEnableTracing(?bool $enableTracing): self
     {
@@ -144,6 +146,8 @@ public function setEnableTracing(?bool $enableTracing): self
      * Gets if tracing is enabled or not.
      *
      * @return bool|null If the option `enable_tracing` is set or not
+     *
+     * @deprecated since version 4.7. To be removed in version 5.0
      */
     public function getEnableTracing(): ?bool
     {
@@ -1100,7 +1104,7 @@ private function configureOptions(OptionsResolver $resolver): void
             'logger' => null,
             'spotlight' => false,
             'spotlight_url' => 'http://localhost:8969',
-            'release' => $_SERVER['SENTRY_RELEASE'] ?? null,
+            'release' => $_SERVER['SENTRY_RELEASE'] ?? $_SERVER['AWS_LAMBDA_FUNCTION_VERSION'] ?? null,
             'dsn' => $_SERVER['SENTRY_DSN'] ?? null,
             'server_name' => gethostname(),
             'ignore_exceptions' => [],

diff --git a/src/Serializer/AbstractSerializer.php b/src/Serializer/AbstractSerializer.php
@@ -261,7 +261,7 @@ protected function serializeValue($value)
         }
 
         try {
-            if (\is_callable($value)) {
+            if (@\is_callable($value)) {
                 return $this->serializeCallable($value);
             }
         } catch (\Throwable $exception) {

diff --git a/src/Serializer/EnvelopItems/MetricsItem.php b/src/Serializer/EnvelopItems/MetricsItem.php
@@ -6,6 +6,7 @@
 
 use Sentry\Event;
 use Sentry\Metrics\MetricsUnit;
+use Sentry\Metrics\Types\AbstractType;
 use Sentry\Serializer\Traits\StacktraceFrameSeralizerTrait;
 use Sentry\Util\JSON;
 
@@ -19,12 +20,17 @@ class MetricsItem implements EnvelopeItemInterface
     /**
      * @var string
      */
-    private const KEY_PATTERN = '/[^a-zA-Z0-9_\/.-]+/i';
+    private const KEY_PATTERN = '/[^\w\-.]+/i';
 
     /**
      * @var string
      */
-    private const VALUE_PATTERN = '/[^\w\d\s_:\/@\.{}\[\]$-]+/i';
+    private const UNIT_PATTERN = '/[^\w]+/i';
+
+    /**
+     * @var string
+     */
+    private const TAG_KEY_PATTERN = '/[^\w\-.\/]+/i';
 
     public static function toEnvelopeItem(Event $event): string
     {
@@ -37,37 +43,7 @@ public static function toEnvelopeItem(Event $event): string
         $metricMetaPayload = [];
 
         foreach ($metrics as $metric) {
-            // key - my.metric
-            $line = preg_replace(self::KEY_PATTERN, '_', $metric->getKey());
-
-            if ($metric->getUnit() !== MetricsUnit::none()) {
-                // unit - @second
-                $line .= '@' . $metric->getunit();
-            }
-
-            foreach ($metric->serialize() as $value) {
-                // value - 2:3:4...
-                $line .= ':' . $value;
-            }
-
-            // type - |c|, |d|, ...
-            $line .= '|' . $metric->getType() . '|';
-
-            $tags = [];
-            foreach ($metric->getTags() as $key => $value) {
-                $tags[] = preg_replace(self::KEY_PATTERN, '_', $key) .
-                    ':' . preg_replace(self::VALUE_PATTERN, '', $value);
-            }
-
-            if (!empty($tags)) {
-                // tags - #key:value,key:value...
-                $line .= '#' . implode(',', $tags) . '|';
-            }
-
-            // timestamp - T123456789
-            $line .= 'T' . $metric->getTimestamp();
-
-            $statsdPayload[] = $line;
+            $statsdPayload[] = self::seralizeMetric($metric);
 
             if ($metric->hasCodeLocation()) {
                 $metricMetaPayload[$metric->getMri()][] = array_merge(
@@ -110,4 +86,75 @@ public static function toEnvelopeItem(Event $event): string
             $statsdPayload
         );
     }
+
+    public static function seralizeMetric(AbstractType $metric): string
+    {
+        /**
+         * In case of us adding support for emitting metrics from other namespaces,
+         * we have to alter the RateLimiter::class to properly handle these
+         * namespaces.
+         */
+
+        // key - my.metric
+        $line = preg_replace(self::KEY_PATTERN, '_', $metric->getKey());
+
+        if ($metric->getUnit() !== MetricsUnit::none()) {
+            // unit - @second
+            $line .= '@' . preg_replace(self::UNIT_PATTERN, '', (string) $metric->getUnit());
+        }
+
+        foreach ($metric->serialize() as $value) {
+            // value - 2:3:4...
+            $line .= ':' . $value;
+        }
+
+        // type - |c|, |d|, ...
+        $line .= '|' . $metric->getType() . '|';
+
+        $tags = [];
+        foreach ($metric->getTags() as $key => $value) {
+            $tags[] = preg_replace(self::TAG_KEY_PATTERN, '', $key) .
+                ':' . self::escapeTagValues($value);
+        }
+
+        if (!empty($tags)) {
+            // tags - #key:value,key:value...
+            $line .= '#' . implode(',', $tags) . '|';
+        }
+
+        // timestamp - T123456789
+        $line .= 'T' . $metric->getTimestamp();
+
+        return $line;
+    }
+
+    public static function escapeTagValues(string $tagValue): string
+    {
+        $result = '';
+
+        for ($i = 0; $i < mb_strlen($tagValue); ++$i) {
+            $character = mb_substr($tagValue, $i, 1);
+            $result .= str_replace(
+                [
+                    "\n",
+                    "\r",
+                    "\t",
+                    '\\',
+                    '|',
+                    ',',
+                ],
+                [
+                    '\n',
+                    '\r',
+                    '\t',
+                    '\\\\',
+                    '\u{7c}',
+                    '\u{2c}',
+                ],
+                $character
+            );
+        }
+
+        return $result;
+    }
 }
diff --git a/src/Tracing/GuzzleTracingMiddleware.php b/src/Tracing/GuzzleTracingMiddleware.php
@@ -48,14 +48,22 @@ public static function trace(?HubInterface $hub = null): \Closure
                     'path' => $request->getUri()->getPath(),
                 ]);
 
+                $spanAndBreadcrumbData = [
+                    'http.request.method' => $request->getMethod(),
+                    'http.request.body.size' => $request->getBody()->getSize(),
+                ];
+
+                if ($request->getUri()->getQuery() !== '') {
+                    $spanAndBreadcrumbData['http.query'] = $request->getUri()->getQuery();
+                }
+                if ($request->getUri()->getFragment() !== '') {
+                    $spanAndBreadcrumbData['http.fragment'] = $request->getUri()->getFragment();
+                }
+
                 $spanContext = new SpanContext();
                 $spanContext->setOp('http.client');
-                $spanContext->setDescription($request->getMethod() . ' ' . (string) $partialUri);
-                $spanContext->setData([
-                    'http.request.method' => $request->getMethod(),
-                    'http.query' => $request->getUri()->getQuery(),
-                    'http.fragment' => $request->getUri()->getFragment(),
-                ]);
+                $spanContext->setDescription($request->getMethod() . ' ' . $partialUri);
+                $spanContext->setData($spanAndBreadcrumbData);
 
                 $childSpan = $span->startChild($spanContext);
 
@@ -66,7 +74,7 @@ public static function trace(?HubInterface $hub = null): \Closure
                         ->withHeader('baggage', $childSpan->toBaggage());
                 }
 
-                $handlerPromiseCallback = static function ($responseOrException) use ($hub, $request, $childSpan, $partialUri) {
+                $handlerPromiseCallback = static function ($responseOrException) use ($hub, $spanAndBreadcrumbData, $childSpan, $partialUri) {
                     // We finish the span (which means setting the span end timestamp) first to ensure the measured time
                     // the span spans is as close to only the HTTP request time and do the data collection afterwards
                     $childSpan->finish();
@@ -80,23 +88,12 @@ public static function trace(?HubInterface $hub = null): \Closure
                         $response = $responseOrException->getResponse();
                     }
 
-                    $breadcrumbData = [
-                        'url' => (string) $partialUri,
-                        'http.request.method' => $request->getMethod(),
-                        'http.request.body.size' => $request->getBody()->getSize(),
-                    ];
-                    if ($request->getUri()->getQuery() !== '') {
-                        $breadcrumbData['http.query'] = $request->getUri()->getQuery();
-                    }
-                    if ($request->getUri()->getFragment() !== '') {
-                        $breadcrumbData['http.fragment'] = $request->getUri()->getFragment();
-                    }
-
                     if ($response !== null) {
-                        $childSpan->setStatus(SpanStatus::createFromHttpStatusCode($response->getStatusCode()));
+                        $spanAndBreadcrumbData['http.response.body.size'] = $response->getBody()->getSize();
+                        $spanAndBreadcrumbData['http.response.status_code'] = $response->getStatusCode();
 
-                        $breadcrumbData['http.response.status_code'] = $response->getStatusCode();
-                        $breadcrumbData['http.response.body.size'] = $response->getBody()->getSize();
+                        $childSpan->setStatus(SpanStatus::createFromHttpStatusCode($response->getStatusCode()));
+                        $childSpan->setData($spanAndBreadcrumbData);
                     } else {
                         $childSpan->setStatus(SpanStatus::internalError());
                     }
@@ -106,7 +103,9 @@ public static function trace(?HubInterface $hub = null): \Closure
                         Breadcrumb::TYPE_HTTP,
                         'http',
                         null,
-                        $breadcrumbData
+                        array_merge([
+                            'url' => (string) $partialUri,
+                        ], $spanAndBreadcrumbData)
                     ));
 
                     if ($responseOrException instanceof \Throwable) {

diff --git a/src/Tracing/SpanStatus.php b/src/Tracing/SpanStatus.php
@@ -76,11 +76,22 @@ public static function failedPrecondition(): self
      * Gets an instance of this enum representing the fact that the server returned
      * 429 Too Many Requests.
      */
-    public static function resourceExchausted(): self
+    public static function resourceExhausted(): self
     {
         return self::getInstance('resource_exhausted');
     }
 
+    /**
+     * Gets an instance of this enum representing the fact that the server returned
+     * 429 Too Many Requests.
+     *
+     * @deprecated since version 4.7. To be removed in version 5.0. Use SpanStatus::resourceExhausted() instead.
+     */
+    public static function resourceExchausted(): self
+    {
+        return self::resourceExhausted();
+    }
+
     /**
      * Gets an instance of this enum representing the fact that the server returned
      * 501 Not Implemented.
@@ -163,7 +174,7 @@ public static function createFromHttpStatusCode(int $statusCode): self
             case $statusCode === 413:
                 return self::failedPrecondition();
             case $statusCode === 429:
-                return self::resourceExchausted();
+                return self::resourceExhausted();
             case $statusCode === 501:
                 return self::unimplemented();
             case $statusCode === 503: