From 50c88eb432b4a0880484a4db46165ad431d12a14 Mon Sep 17 00:00:00 2001 From: Cari Spruiell Date: Thu, 26 Aug 2021 16:48:38 -0500 Subject: [PATCH] PWA-1311: New Relic is not being given useful transaction names for graphql requests - pull query analysis into helper, add tests --- .../Magento/GraphQl/Controller/GraphQl.php | 92 +---- .../GraphQl/Helper/Query/Logger/LogData.php | 88 ++++ .../Model/Query/Logger/LoggerInterface.php | 2 +- .../GraphQl/Model/Query/Logger/LoggerPool.php | 8 - .../Helper/Query/Logger/LogDataTest.php | 385 ++++++++++++++++++ 5 files changed, 487 insertions(+), 88 deletions(-) create mode 100644 app/code/Magento/GraphQl/Helper/Query/Logger/LogData.php create mode 100644 dev/tests/integration/testsuite/Magento/GraphQl/Helper/Query/Logger/LogDataTest.php diff --git a/app/code/Magento/GraphQl/Controller/GraphQl.php b/app/code/Magento/GraphQl/Controller/GraphQl.php index 935da49ee956c..a81f3bd352423 100644 --- a/app/code/Magento/GraphQl/Controller/GraphQl.php +++ b/app/code/Magento/GraphQl/Controller/GraphQl.php @@ -7,12 +7,6 @@ namespace Magento\GraphQl\Controller; -use GraphQL\Error\SyntaxError; -use GraphQL\Language\AST\Node; -use GraphQL\Language\AST\NodeKind; -use GraphQL\Language\Parser; -use GraphQL\Language\Source; -use GraphQL\Language\Visitor; use Magento\Framework\App\FrontControllerInterface; use Magento\Framework\App\ObjectManager; use Magento\Framework\App\Request\Http; @@ -27,8 +21,8 @@ use Magento\Framework\GraphQl\Schema\SchemaGeneratorInterface; use Magento\Framework\Serialize\SerializerInterface; use Magento\Framework\Webapi\Response; +use Magento\GraphQl\Helper\Query\Logger\LogData; use Magento\GraphQl\Model\Query\ContextFactoryInterface; -use Magento\GraphQl\Model\Query\Logger\LoggerInterface; use Magento\GraphQl\Model\Query\Logger\LoggerPool; /** @@ -97,6 +91,11 @@ class GraphQl implements FrontControllerInterface */ private $contextFactory; + /** + * @var LogData + */ + private $logDataHelper; + /** * @var LoggerPool */ @@ -114,6 +113,7 @@ class GraphQl implements FrontControllerInterface * @param JsonFactory|null $jsonFactory * @param HttpResponse|null $httpResponse * @param ContextFactoryInterface $contextFactory + * @param LogData $logDataHelper * @param LoggerPool $loggerPool * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ @@ -129,6 +129,7 @@ public function __construct( JsonFactory $jsonFactory = null, HttpResponse $httpResponse = null, ContextFactoryInterface $contextFactory = null, + LogData $logDataHelper = null, LoggerPool $loggerPool = null ) { $this->response = $response; @@ -142,6 +143,7 @@ public function __construct( $this->jsonFactory = $jsonFactory ?: ObjectManager::getInstance()->get(JsonFactory::class); $this->httpResponse = $httpResponse ?: ObjectManager::getInstance()->get(HttpResponse::class); $this->contextFactory = $contextFactory ?: ObjectManager::getInstance()->get(ContextFactoryInterface::class); + $this->logDataHelper = $logDataHelper ?: ObjectManager::getInstance()->get(LogData::class); $this->loggerPool = $loggerPool ?: ObjectManager::getInstance()->get(LoggerPool::class); } @@ -168,11 +170,12 @@ public function dispatch(RequestInterface $request): ResponseInterface // Temporal coupling is required for performance optimization $this->queryFields->setQuery($query, $variables); - // log information about the query - $this->logQueryInformation($request, $data); - $schema = $this->schemaGenerator->generate(); + // log information about the query + $queryInformation = $this->logDataHelper->getQueryInformation($request, $data, $schema); + $this->loggerPool->execute($queryInformation); + $result = $this->queryProcessor->process( $schema, $query, @@ -214,73 +217,4 @@ private function getDataFromRequest(RequestInterface $request): array return $data; } - - /** - * Logs information about the query - * - * @param RequestInterface $request - * @param array $data - * @throws SyntaxError - */ - private function logQueryInformation(RequestInterface $request, array $data) - { - $query = $data['query'] ?? ''; - $queryInformation = []; - $queryInformation[LoggerInterface::HTTP_METHOD] = $request->getMethod(); - $queryInformation[LoggerInterface::STORE_HEADER] = $request->getHeader('Store', ''); - $queryInformation[LoggerInterface::CURRENCY_HEADER] = $request->getHeader('Currency', ''); - $queryInformation[LoggerInterface::AUTH_HEADER_SET] = $request->getHeader('Authorization') ? 'true' : 'false'; - $queryInformation[LoggerInterface::IS_CACHEABLE] = $request->getHeader('X-Magento-Cache-Id') ? 'true' : 'false'; - $queryInformation[LoggerInterface::NUMBER_OF_QUERIES] = ''; - $queryInformation[LoggerInterface::QUERY_NAMES] = $this->getOperationName($data); - $queryInformation[LoggerInterface::HAS_MUTATION] = str_contains($query, 'mutation') ? 'true' : 'false'; - $queryInformation[LoggerInterface::QUERY_COMPLEXITY] = $this->getFieldCount($query); - $queryInformation[LoggerInterface::QUERY_LENGTH] = $request->getHeader('Content-Length'); - - $this->loggerPool->execute($queryInformation); - } - - /** - * Get GraphQL query operation name - * - * @param array $data - * @return string - */ - private function getOperationName(array $data): string - { - if (isset($data['operationName']) && is_string($data['operationName']) && $data['operationName'] !== '') { - return $data['operationName']; - } - - $fields = $this->queryFields->getFieldsUsedInQuery(); - return current($fields) ?: 'operationNameNotFound'; - } - - /** - * Gets the field count - * - * @param string $query - * @return int - * @throws SyntaxError - * @throws \Exception - */ - private function getFieldCount(string $query): int - { - if (!empty($query)) { - $totalFieldCount = 0; - $queryAst = Parser::parse(new Source($query ?: '', 'GraphQL')); - Visitor::visit( - $queryAst, - [ - 'leave' => [ - NodeKind::FIELD => function (Node $node) use (&$totalFieldCount) { - $totalFieldCount++; - } - ] - ] - ); - return $totalFieldCount; - } - return 0; - } } diff --git a/app/code/Magento/GraphQl/Helper/Query/Logger/LogData.php b/app/code/Magento/GraphQl/Helper/Query/Logger/LogData.php new file mode 100644 index 0000000000000..af6b96174a0a6 --- /dev/null +++ b/app/code/Magento/GraphQl/Helper/Query/Logger/LogData.php @@ -0,0 +1,88 @@ +getMethod(); + $queryInformation[LoggerInterface::STORE_HEADER] = $request->getHeader('Store') ?: ''; + $queryInformation[LoggerInterface::CURRENCY_HEADER] = $request->getHeader('Currency') ?: ''; + $queryInformation[LoggerInterface::HAS_AUTH_HEADER] = $request->getHeader('Authorization') ? 'true' : 'false'; + $queryInformation[LoggerInterface::IS_CACHEABLE] = $request->getHeader('X-Magento-Cache-Id') ? 'true' : 'false'; + $queryInformation[LoggerInterface::QUERY_LENGTH] = $request->getHeader('Content-Length') ?: ''; + + $schemaConfig = $schema->getConfig(); + $configMutationFields = $schemaConfig->getMutation()->getFields(); + $configQueryFields = $schemaConfig->getQuery()->getFields(); + $queryInformation[LoggerInterface::HAS_MUTATION] = count($configMutationFields) > 0 ? 'true' : 'false'; + $queryInformation[LoggerInterface::NUMBER_OF_QUERIES] = + count($configMutationFields) + count($configQueryFields); + + $queryNames = array_merge(array_keys($configMutationFields), array_keys($configQueryFields)); + $queryInformation[LoggerInterface::QUERY_NAMES] = + count($queryNames) > 0 ? implode(", ", $queryNames) : 'operationNameNotFound'; + $queryInformation[LoggerInterface::QUERY_COMPLEXITY] = $this->getFieldCount($query); + + return $queryInformation; + } + + /** + * Gets the field count + * + * @param string $query + * @return int + * @throws SyntaxError + * @throws \Exception + */ + private function getFieldCount(string $query): int + { + if (!empty($query)) { + $totalFieldCount = 0; + $queryAst = Parser::parse(new Source($query ?: '', 'GraphQL')); + Visitor::visit( + $queryAst, + [ + 'leave' => [ + NodeKind::FIELD => function (Node $node) use (&$totalFieldCount) { + $totalFieldCount++; + } + ] + ] + ); + return $totalFieldCount; + } + return 0; + } +} diff --git a/app/code/Magento/GraphQl/Model/Query/Logger/LoggerInterface.php b/app/code/Magento/GraphQl/Model/Query/Logger/LoggerInterface.php index c51e1ab8ea1bf..cc1b788fad9d0 100644 --- a/app/code/Magento/GraphQl/Model/Query/Logger/LoggerInterface.php +++ b/app/code/Magento/GraphQl/Model/Query/Logger/LoggerInterface.php @@ -18,7 +18,7 @@ interface LoggerInterface const QUERY_NAMES = 'GraphQlQueryNames'; const STORE_HEADER = 'GraphQlStoreHeader'; const CURRENCY_HEADER = 'GraphQlCurrencyHeader'; - const AUTH_HEADER_SET = 'GraphQlAuthHeaderSet'; + const HAS_AUTH_HEADER = 'GraphQlHasAuthHeader'; const HTTP_METHOD = 'GraphQlHttpMethod'; const HAS_MUTATION = 'GraphQlHasMutation'; const IS_CACHEABLE = 'GraphQlIsCacheable'; diff --git a/app/code/Magento/GraphQl/Model/Query/Logger/LoggerPool.php b/app/code/Magento/GraphQl/Model/Query/Logger/LoggerPool.php index 65cd495e0a335..dbb7d6381bb03 100644 --- a/app/code/Magento/GraphQl/Model/Query/Logger/LoggerPool.php +++ b/app/code/Magento/GraphQl/Model/Query/Logger/LoggerPool.php @@ -19,19 +19,11 @@ class LoggerPool implements LoggerInterface private $loggers; /** - * @var LoggerInterfaceFactory - */ - private $loggerFactory; - - /** - * @param LoggerInterfaceFactory $loggerFactory * @param LoggerInterface[] $loggers */ public function __construct( - LoggerInterfaceFactory $loggerFactory, $loggers = [] ) { - $this->loggerFactory = $loggerFactory; $this->loggers = $loggers; } diff --git a/dev/tests/integration/testsuite/Magento/GraphQl/Helper/Query/Logger/LogDataTest.php b/dev/tests/integration/testsuite/Magento/GraphQl/Helper/Query/Logger/LogDataTest.php new file mode 100644 index 0000000000000..3068f308988a8 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/GraphQl/Helper/Query/Logger/LogDataTest.php @@ -0,0 +1,385 @@ +objectManager = Bootstrap::getObjectManager(); + $this->logData = $this->objectManager->get(LogData::class); + $this->schemaGenerator = $this->objectManager->get(SchemaGenerator::class); + $this->request = $this->objectManager->get(Http::class); + } + + /** + * Test a graphql query is parsed correctly for logging + * + * @param string $query + * @param array $headers + * @param array $expectedResult + * @dataProvider getQueryInformationDataProvider + * @return void + */ + public function testGetQueryInformation(string $query, array $headers, array $expectedResult): void + { + $this->request->setPathInfo('/graphql'); + $this->request->setMethod('POST'); + $postData = [ + 'query' => $query, + 'variables' => null, + 'operationName' => null + ]; + $this->request->setContent(json_encode($postData)); + + $requestHeaders = $this->objectManager->create(Headers::class)->addHeaders($headers); + $this->request->setHeaders($requestHeaders); + + $myschemaGenerator = $this->objectManager->get(SchemaGenerator::class); + $queryFields = $this->objectManager->get(Fields::class); + $queryFields->setQuery($query); + + $queryInformation = $this->logData->getQueryInformation( + $this->request, + $postData, + $myschemaGenerator->generate()); + + $this->assertEquals($expectedResult, $queryInformation); + } + + public function getQueryInformationDataProvider() + { + return [ + [ // query with all headers + 'query' => << [ + 'Store' => 1, + 'Currency' => 'USD', + 'Authorization' => '1234', + 'X-Magento-Cache-Id' => true, + 'Content-length' => 123 + ], + 'expectedResult' => [ + LoggerInterface::HTTP_METHOD => 'POST', + LoggerInterface::STORE_HEADER => 1, + LoggerInterface::CURRENCY_HEADER => 'USD', + LoggerInterface::HAS_AUTH_HEADER => 'true', + LoggerInterface::IS_CACHEABLE => 'true', + LoggerInterface::QUERY_LENGTH => 123, + LoggerInterface::HAS_MUTATION => 'false', + LoggerInterface::NUMBER_OF_QUERIES => 1, + LoggerInterface::QUERY_NAMES => 'products', + LoggerInterface::QUERY_COMPLEXITY => 5 + ] + ], + [ // multiple query + 'query' => << [ + ], + 'expectedResult' => [ + LoggerInterface::HTTP_METHOD => 'POST', + LoggerInterface::STORE_HEADER => '', + LoggerInterface::CURRENCY_HEADER => '', + LoggerInterface::HAS_AUTH_HEADER => 'false', + LoggerInterface::IS_CACHEABLE => 'false', + LoggerInterface::QUERY_LENGTH => '', + LoggerInterface::HAS_MUTATION => 'false', + LoggerInterface::NUMBER_OF_QUERIES => 2, + LoggerInterface::QUERY_NAMES => 'products,cart', + LoggerInterface::QUERY_COMPLEXITY => 28 + ] + ], + [ // query with no headers + 'query' => << [], + 'expectedResult' => [ + LoggerInterface::HTTP_METHOD => 'POST', + LoggerInterface::STORE_HEADER => '', + LoggerInterface::CURRENCY_HEADER => '', + LoggerInterface::HAS_AUTH_HEADER => 'false', + LoggerInterface::IS_CACHEABLE => 'false', + LoggerInterface::QUERY_LENGTH => '', + LoggerInterface::HAS_MUTATION => 'false', + LoggerInterface::NUMBER_OF_QUERIES => 1, + LoggerInterface::QUERY_NAMES => 'products', + LoggerInterface::QUERY_COMPLEXITY => 5 + ] + ], + [ // mutation with all headers + 'query' => << [ + 'Store' => 1, + 'Currency' => 'USD', + 'Authorization' => '1234', + 'X-Magento-Cache-Id' => true, + 'Content-length' => 123 + ], + 'expectedResult' => [ + LoggerInterface::HTTP_METHOD => 'POST', + LoggerInterface::STORE_HEADER => '1', + LoggerInterface::CURRENCY_HEADER => 'USD', + LoggerInterface::HAS_AUTH_HEADER => 'true', + LoggerInterface::IS_CACHEABLE => 'true', + LoggerInterface::QUERY_LENGTH => '123', + LoggerInterface::HAS_MUTATION => 'false', + LoggerInterface::NUMBER_OF_QUERIES => 1, + LoggerInterface::QUERY_NAMES => 'placeOrder', + LoggerInterface::QUERY_COMPLEXITY => 3 + ] + ], + ]; + } + + /** + * Test request is dispatched and response generated when using GET request with query string + * + * @return void + */ + public function testDispatchWithGet(): void + { + /** @var ProductRepositoryInterface $productRepository */ + $productRepository = $this->objectManager->get(ProductRepositoryInterface::class); + + /** @var ProductInterface $product */ + $product = $productRepository->get('simple1'); + + $query + = <<request->setPathInfo('/graphql'); + $this->request->setMethod('GET'); + $this->request->setQueryValue('query', $query); + $response = $this->graphql->dispatch($this->request); + $output = $this->jsonSerializer->unserialize($response->getContent()); + $linkField = $this->metadataPool->getMetadata(ProductInterface::class)->getLinkField(); + + $this->assertArrayNotHasKey('errors', $output, 'Response has errors'); + $this->assertNotEmpty($output['data']['products']['items'], 'Products array has items'); + $this->assertNotEmpty($output['data']['products']['items'][0], 'Products array has items'); + $this->assertEquals($product->getData($linkField), $output['data']['products']['items'][0]['id']); + $this->assertEquals($product->getSku(), $output['data']['products']['items'][0]['sku']); + $this->assertEquals($product->getName(), $output['data']['products']['items'][0]['name']); + } + + /** Test request is dispatched and response generated when using GET request with parameterized query string + * + * @return void + */ + public function testDispatchGetWithParameterizedVariables(): void + { + /** @var ProductRepositoryInterface $productRepository */ + $productRepository = $this->objectManager->get(ProductRepositoryInterface::class); + + /** @var ProductInterface $product */ + $product = $productRepository->get('simple1'); + $query = << [ + 'sku' => ['eq' => 'simple1'] + ] + ]; + $queryParams = [ + 'query' => $query, + 'variables' => json_encode($variables), + 'operationName' => 'GetProducts' + ]; + + $this->request->setPathInfo('/graphql'); + $this->request->setMethod('GET'); + $this->request->setParams($queryParams); + $response = $this->graphql->dispatch($this->request); + $output = $this->jsonSerializer->unserialize($response->getContent()); + $linkField = $this->metadataPool->getMetadata(ProductInterface::class)->getLinkField(); + + $this->assertArrayNotHasKey('errors', $output, 'Response has errors'); + $this->assertNotEmpty($output['data']['products']['items'], 'Products array has items'); + $this->assertNotEmpty($output['data']['products']['items'][0], 'Products array has items'); + $this->assertEquals($product->getData($linkField), $output['data']['products']['items'][0]['id']); + $this->assertEquals($product->getSku(), $output['data']['products']['items'][0]['sku']); + $this->assertEquals($product->getName(), $output['data']['products']['items'][0]['name']); + } + + /** + * Test the errors on graphql output + * + * @return void + */ + public function testError(): void + { + $query + = << $query, + 'variables' => null, + 'operationName' => null + ]; + + $this->request->setPathInfo('/graphql'); + $this->request->setMethod('POST'); + $this->request->setContent(json_encode($postData)); + $headers = $this->objectManager->create(\Laminas\Http\Headers::class) + ->addHeaders(['Content-Type' => 'application/json']); + $this->request->setHeaders($headers); + $response = $this->graphql->dispatch($this->request); + $outputResponse = $this->jsonSerializer->unserialize($response->getContent()); + if (isset($outputResponse['errors'][0])) { + if (is_array($outputResponse['errors'][0])) { + foreach ($outputResponse['errors'] as $error) { + $this->assertEquals( + \Magento\Framework\GraphQl\Exception\GraphQlInputException::EXCEPTION_CATEGORY, + $error['extensions']['category'] + ); + if (isset($error['message'])) { + $this->assertEquals($error['message'], 'Invalid entity_type specified: invalid'); + } + if (isset($error['trace'])) { + if (is_array($error['trace'])) { + $this->assertNotEmpty($error['trace']); + } + } + } + } + } + } +}