Merge branch 'deprecate-sample-player' into 'main'

Deprecate the samples attribute in .blackfire.yml file

See merge request platformsh/observability/blackfire/blackfire.io!33974

Player/Extension/BlackfireExtension.php

@@ -216,8 +216,6 @@ private function createBuildProfileConfig(Step $step, StepContext $stepContext,
     {
         $config = new ProfileConfiguration();
 
-        $config->setSamples($this->language->evaluate($stepContext->getSamples(), $context->getVariableValues($stepContext, true)));
-
         $path = parse_url($request->uri, \PHP_URL_PATH) ?: '/';
         $config->setTitle($this->language->evaluate($step->getName() ?: Json::encode(sprintf('%s resource', $path)), $context->getVariableValues($stepContext, true)));
 
@@ -323,9 +321,7 @@ private function warmupCount(StepContext $stepContext, string $requestMethod, ar
             return 0;
         }
 
-        $samples = (int) $this->language->evaluate($stepContext->getSamples(), $contextVariables);
-
-        if (\in_array($requestMethod, ['GET', 'HEAD'], true) || $samples > 1) {
+        if (\in_array($requestMethod, ['GET', 'HEAD'], true)) {
             return true === $value ? 3 : (int) $value;
         }
 

Player/Parser.php

@@ -553,11 +553,7 @@ private function parseStepConfig(Input $input, AbstractStep $step, int $expected
             } elseif ('json' === $keyword) {
                 $step->json(null !== $arguments ? $this->checkExpression($input, $arguments) : 'true');
             } elseif ('samples' === $keyword) {
-                if (null === $arguments) {
-                    throw new SyntaxErrorException(sprintf('A "samples" takes a number as a required argument %s.', $input->getContextString()));
-                }
-
-                $step->samples($this->checkExpression($input, $arguments));
+                $step->addDeprecation('The "samples" attribute has no effect, is deprecated and will be removed in version 3. Remove it from your configuration.');
             } elseif ('warmup' === $keyword) {
                 $step->warmup(null !== $arguments ? $this->checkExpression($input, $arguments) : 'true');
             } elseif ('body' === $keyword) {

Player/Step/ConfigurableStep.php

@@ -76,6 +76,8 @@ public function blackfire(string|null $env): self
 
     public function samples(string|null $samples): self
     {
+        @trigger_error(sprintf('The method "%s" is deprecated since player 2.6 and will be removed in 3.0.', __METHOD__), \E_USER_DEPRECATED);
+
         $this->samples = $samples;
 
         return $this;
@@ -135,7 +137,9 @@ public function isBlackfireEnabled(): bool|null
 
     public function getSamples(): string|null
     {
-        return $this->samples;
+        @trigger_error(sprintf('The method "%s" is deprecated since player 2.6 and will be removed in 3.0.', __METHOD__), \E_USER_DEPRECATED);
+
+        return null !== $this->samples ? 1 : null;
     }
 
     public function getWarmup(): string|null

Player/Step/ReloadStep.php

@@ -30,7 +30,6 @@ public function configureFromStep(AbstractStep $step): void
     {
         if ($step instanceof ConfigurableStep) {
             $this
-                ->samples($step->getSamples())
                 ->blackfire($step->getBlackfire())
                 ->wait($step->getWait())
                 ->json($step->isJson())

Player/Step/StepContext.php

@@ -28,7 +28,6 @@ final class StepContext
     /** @var mixed[] */
     private array $variables = [];
     private string|null $blackfire = null;
-    private string|null $samples = null;
     private string|null $warmup = null;
     private string|null $workingDir = null;
 
@@ -60,10 +59,6 @@ public function update(ConfigurableStep $step, array $variables): void
             $this->blackfire = $step->getBlackfire();
         }
 
-        if (null !== $step->getSamples()) {
-            $this->samples = $step->getSamples();
-        }
-
         if (null !== $step->getWarmup()) {
             $this->warmup = $step->getWarmup();
         }
@@ -130,11 +125,6 @@ public function getBlackfireEnv(): string|null
         return $this->blackfire;
     }
 
-    public function getSamples(): string
-    {
-        return null === $this->samples ? '1' : $this->samples;
-    }
-
     public function getWarmup(): string
     {
         return null === $this->warmup ? 'true' : $this->warmup;

Player/Tests/Extension/BlackfireExtensionTest.php

@@ -534,45 +534,22 @@ public function getPreviousStepsProvider()
             new HttpRequest('HEAD', 'https://app.lan'), [],
         ];
 
-        $visitStepWith2WarmupsNoSamplesAndPostRequest = new VisitStep('https://app.lan');
-        $visitStepWith2WarmupsNoSamplesAndPostRequest->blackfire('true');
-        $visitStepWith2WarmupsNoSamplesAndPostRequest->name('"Visit page"');
-        $visitStepWith2WarmupsNoSamplesAndPostRequest->method('"POST"');
-        $visitStepWith2WarmupsNoSamplesAndPostRequest->warmup(2);
+        $visitStepWith2WarmupsAndPostRequest = new VisitStep('https://app.lan');
+        $visitStepWith2WarmupsAndPostRequest->blackfire('true');
+        $visitStepWith2WarmupsAndPostRequest->name('"Visit page"');
+        $visitStepWith2WarmupsAndPostRequest->method('"POST"');
+        $visitStepWith2WarmupsAndPostRequest->warmup(2);
         // no warmup are expected here as it is a POST request without having set sample > 1
         yield 'VisitStep with POST request and 2 warmups' => [
-            $visitStepWith2WarmupsNoSamplesAndPostRequest,
+            $visitStepWith2WarmupsAndPostRequest,
             [],
             new HttpRequest('POST', 'https://app.lan'), [],
         ];
 
-        $visitStepWith2Warmups5SamplesAndPostRequest = new VisitStep('https://app.lan');
-        $visitStepWith2Warmups5SamplesAndPostRequest->blackfire('true');
-        $visitStepWith2Warmups5SamplesAndPostRequest->name('"Visit page"');
-        $visitStepWith2Warmups5SamplesAndPostRequest->method('"POST"');
-        $visitStepWith2Warmups5SamplesAndPostRequest->warmup(2);
-        $visitStepWith2Warmups5SamplesAndPostRequest->samples(5); // edge case
-        yield 'VisitStep with POST request and 2 warmups and 5 samples' => [
-            $visitStepWith2Warmups5SamplesAndPostRequest,
-            [
-                [
-                    'name' => '"[Warmup] Visit page"',
-                ],
-                [
-                    'name' => '"[Warmup] Visit page"',
-                ],
-                [
-                    'name' => '"[Reference] Visit page"',
-                ],
-            ],
-            new HttpRequest('POST', 'https://app.lan'), [],
-        ];
-
         $visitStepWithoutBlackfire = new VisitStep('https://app.lan');
         $visitStepWithoutBlackfire->blackfire('false');
         $visitStepWithoutBlackfire->name('"Visit page"');
         $visitStepWithoutBlackfire->warmup(2);
-        $visitStepWithoutBlackfire->samples(5);
         yield 'VisitStep without blackfire' => [
             $visitStepWithoutBlackfire,
             [],

Player/Tests/Mock/mockedProbeEndpoint.php

@@ -14,31 +14,11 @@
 const HEADER_BLACKFIRE_QUERY = 'x-blackfire-query';
 const HEADER_BLACKFIRE_RESPONSE = 'x-blackfire-response';
 
-function buildBkfResponseHeader(array &$mockState, array $blackfireQuery, string $endpoint): string
+function buildBkfResponseHeader(): string
 {
-    // let's mock the fact that we correctly computed one sample
-    ++$mockState['endpoints'][$endpoint]['performed_samples'];
-    $performedSamples = $mockState['endpoints'][$endpoint]['performed_samples'];
-
-    $aggregSamples = $blackfireQuery['aggreg_samples'] ?? 1;
-    $completion = (100 * $performedSamples) / $aggregSamples;
-
-    $profileContinues = 'true';
-
-    if ($performedSamples >= $aggregSamples) {
-        $profileContinues = 'false';
-    }
-
-    $blackfireResponse = [
-        'continue' => $profileContinues,
-    ];
-
-    if ('true' === $profileContinues) {
-        $blackfireResponse['progress'] = $completion;
-        $blackfireResponse['wait'] = 0;
-    }
-
-    return http_build_query($blackfireResponse);
+    return http_build_query([
+        'continue' => 'false',
+    ]);
 }
 
 function mockedProbeEndpoint(callable $responseFactory)
@@ -60,7 +40,6 @@ function mockedProbeEndpoint(callable $responseFactory)
     if (!isset($mockState['endpoints'][$endpoint])) {
         $mockState['endpoints'][$endpoint] = [
             'called_times' => 0,
-            'performed_samples' => 0,
         ];
     }
 
@@ -71,7 +50,7 @@ function mockedProbeEndpoint(callable $responseFactory)
         parse_str($headers[HEADER_BLACKFIRE_QUERY], $blackfireQuery);
 
         // compute a blackfire response header
-        $blackfireResponse = buildBkfResponseHeader($mockState, $blackfireQuery, $endpoint);
+        $blackfireResponse = buildBkfResponseHeader();
 
         // append it to the response
         header(HEADER_BLACKFIRE_RESPONSE.': '.$blackfireResponse);

Player/Tests/ParserTest.php

@@ -94,7 +94,6 @@ public function testParsingGlobalConfiguration()
         $this->assertEquals([
             '"Accept-Language: en-US"',
         ], $scenario->getBlockStep()->getHeaders());
-        $this->assertEquals(10, $scenario->getBlockStep()->getSamples());
 
         /** @var Scenario $scenario */
         $scenario = $scenarioSet->getIterator()[1];
@@ -179,17 +178,17 @@ public function warmupConfigProvider()
     }
 
     /**
-     * @dataProvider provideDocSamples
+     * @dataProvider provideDocExamples
      */
-    public function testDocSamples($input)
+    public function testDocExamples($input)
     {
         $parser = new Parser(new ExpressionLanguage(null, [new LanguageProvider()]));
         $scenarioSet = $parser->parse($input);
 
         $this->assertInstanceOf(ScenarioSet::class, $scenarioSet);
     }
 
-    public function provideDocSamples()
+    public function provideDocExamples()
     {
         yield [<<<'EOF'
 scenario

Player/Tests/Serializer/fixtures/serialized_for_jsonview_with_failures_and_exceptions.json

@@ -9,6 +9,10 @@
                 {
                     "status": "todo",
                     "name": "/competitions",
+                    "deprecations": [
+                        "The \"samples\" attribute has no effect, is deprecated and will be removed in version 3. Remove it from your configuration."
+                    ],
+                    "type": "visit",
                     "failing_expectations": [
                         {
                             "reason": "An expectation failed",
@@ -23,8 +27,7 @@
                                 }
                             ]
                         }
-                    ],
-                    "type": "visit"
+                    ]
                 }
             ]
         },
@@ -38,6 +41,9 @@
                     "errors": [
                         "Something exploded"
                     ],
+                    "deprecations": [
+                        "The \"samples\" attribute has no effect, is deprecated and will be removed in version 3. Remove it from your configuration."
+                    ],
                     "type": "visit"
                 }
             ]

Player/Tests/Serializer/fixtures/test4.json

@@ -13,10 +13,12 @@
                     "method": "'GET'",
                     "uri": "url('https://api.dreamcargiveaways.co.uk/competitions')",
                     "status": "todo",
-                    "samples": "10",
                     "warmup": "true",
                     "is_blackfire_enabled": true,
                     "name": "/competitions",
+                    "deprecations": [
+                        "The \"samples\" attribute has no effect, is deprecated and will be removed in version 3. Remove it from your configuration."
+                    ],
                     "line": 5,
                     "type": "visit"
                 }

Player/Tests/Serializer/fixtures/test5_1.json

@@ -13,10 +13,12 @@
                     "method": "'GET'",
                     "uri": "url('https://api.dreamcargiveaways.co.uk/competitions')",
                     "status": "todo",
-                    "samples": "10",
                     "warmup": "true",
                     "is_blackfire_enabled": true,
                     "name": "/competitions",
+                    "deprecations": [
+                        "The \"samples\" attribute has no effect, is deprecated and will be removed in version 3. Remove it from your configuration."
+                    ],
                     "line": 5,
                     "type": "visit"
                 }
@@ -31,15 +33,17 @@
                     "method": "'GET'",
                     "uri": "url('https://api.dreamcargiveaways.co.uk/competitions')",
                     "status": "todo",
-                    "samples": "10",
                     "warmup": "true",
                     "is_blackfire_enabled": true,
                     "name": "/competitions",
+                    "deprecations": [
+                        "The \"samples\" attribute has no effect, is deprecated and will be removed in version 3. Remove it from your configuration."
+                    ],
                     "line": 13,
                     "type": "visit"
                 }
             ],
             "line": 11
         }
     ]
-}
+}

Player/Tests/Serializer/fixtures/test5_2.json

@@ -13,15 +13,17 @@
                     "method": "'GET'",
                     "uri": "url('https://api.dreamcargiveaways.co.uk/competitions')",
                     "status": "todo",
-                    "samples": "10",
                     "warmup": "true",
                     "is_blackfire_enabled": true,
                     "name": "/competitions",
+                    "deprecations": [
+                        "The \"samples\" attribute has no effect, is deprecated and will be removed in version 3. Remove it from your configuration."
+                    ],
                     "line": 5,
                     "type": "visit"
                 }
             ],
             "line": 3
         }
     ]
-}
+}