Skip to content

Commit

Permalink
Merge pull request #249 from scoutapp/248-promote-http-spans
Browse files Browse the repository at this point in the history
Add HTTP span into file_get_contents and curl_exec
  • Loading branch information
asgrim authored Jan 10, 2022
2 parents 4471586 + 609b6bc commit 4e1396f
Show file tree
Hide file tree
Showing 13 changed files with 320 additions and 36 deletions.
8 changes: 6 additions & 2 deletions .github/workflows/laravel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ jobs:
with:
coverage: "none"
php-version: "${{ matrix.php-version }}"
tools: pecl
tools: pecl, composer:v2.2
extensions: ${{ matrix.extensions }}
env:
fail-fast: true
# --no-update then a full `composer update` is needed to overcome locked dependencies
# See: https://github.com/composer/composer/issues/9561
- name: "Remove existing requirements components (avoid conflicts)"
Expand Down Expand Up @@ -85,8 +87,10 @@ jobs:
with:
coverage: "none"
php-version: "${{ matrix.php-version }}"
tools: pecl
tools: pecl, composer:v2.2
extensions: ${{ matrix.extensions }}
env:
fail-fast: true
- name: "Install Laravel quickstart project"
run: "composer create-project laravel/laravel:${{ matrix.laravel-version}} test-app --prefer-dist"
- name: "Add scout-apm-php as a repository"
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/lumen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ jobs:
with:
coverage: "none"
php-version: "${{ matrix.php-version }}"
tools: pecl
tools: pecl, composer:v2.2
extensions: ${{ matrix.extensions }}
env:
fail-fast: true
# --no-update then a full `composer update` is needed to overcome locked dependencies
# See: https://github.com/composer/composer/issues/9561
- name: "Remove existing requirements components (avoid conflicts)"
Expand Down Expand Up @@ -91,8 +93,10 @@ jobs:
with:
coverage: "none"
php-version: "${{ matrix.php-version }}"
tools: pecl
tools: pecl, composer:v2.2
extensions: ${{ matrix.extensions }}
env:
fail-fast: true
- name: "Install Lumen quickstart project"
run: "composer create-project laravel/lumen:${{ matrix.lumen-version}} test-app --prefer-dist"
- name: "Add scout-apm-php as a repository"
Expand Down
9 changes: 9 additions & 0 deletions .github/workflows/quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ jobs:
with:
coverage: "none"
php-version: "8.0"
tools: composer:v2.2
env:
fail-fast: true
- name: "Install dependencies"
run: "composer install"
- name: "Run PHP_CodeSniffer"
Expand All @@ -34,6 +37,9 @@ jobs:
with:
coverage: "none"
php-version: "8.0"
tools: composer:v2.2
env:
fail-fast: true
- name: "Install dependencies"
run: "composer install"
- name: "Run Psalm"
Expand All @@ -51,6 +57,9 @@ jobs:
with:
coverage: "none"
php-version: "8.0"
tools: composer:v2.2
env:
fail-fast: true
- name: "Require Roave/BackwardCompatibilityCheck"
run: "composer require --no-update --no-interaction --prefer-dist --prefer-stable --dev roave/backward-compatibility-check:^6.0.1"
- name: "Composer update with new requirements"
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/symfony.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ jobs:
with:
coverage: "none"
php-version: "${{ matrix.php-version }}"
tools: pecl
tools: pecl, composer:v2.2
extensions: ${{ matrix.extensions }}
env:
fail-fast: true
# --no-update then a full `composer update` is needed to overcome locked dependencies
# See: https://github.com/composer/composer/issues/9561
- name: "Remove existing requirements components (avoid conflicts)"
Expand Down
18 changes: 14 additions & 4 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ jobs:
fail-fast: false
matrix:
dependencies: ["lowest", "highest"]
scout-ext: ["with-scout-ext", "no-scout-ext"]
extensions: [
"scoutapm",
"scoutapm, mongodb",
"",
"mongodb"
]
Expand All @@ -42,8 +41,19 @@ jobs:
with:
coverage: "none"
php-version: "${{ matrix.php-version }}"
tools: pecl
extensions: ${{ matrix.extensions }}
tools: pecl, composer:v2.2
extensions: "curl, ${{ matrix.extensions }}"
env:
fail-fast: true
# Normally, we'd just add "scoutapm" to the above extensions in shivammathur/setup-php, but libcurl appears to
# be missing wherever the extension is built (not immediately obvious), so install it first
- name: "Install scoutapm extension"
if: ${{ matrix.scout-ext == 'with-scout-ext' }}
run: |
sudo apt-get install -y libcurl4-openssl-dev
sudo mkdir -p /tmp/pear/temp
sudo pecl update-channels
yes | sudo pecl install -f scoutapm
- name: "Install lowest dependencies"
if: ${{ matrix.dependencies == 'lowest' }}
run: "composer update --prefer-lowest --prefer-dist --no-interaction --no-progress"
Expand Down
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,28 @@

All notable changes to this project will be documented in this file, in reverse chronological order by release.

## 6.7.0 - 2022-01-10

### Added

- [#249](https://github.com/scoutapp/scout-apm-php/pull/249) Added HTTP spans for file_get_contents and curl_exec

### Changed

- Nothing.

### Deprecated

- Nothing.

### Removed

- Nothing.

### Fixed

- Nothing.

## 6.6.1 - 2022-01-03

### Added
Expand Down
11 changes: 10 additions & 1 deletion src/Agent.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ final class Agent implements ScoutApmAgent

private const METADATA_CACHE_TTL_SECONDS = 600;

private const WARN_WHEN_EXTENSION_IS_OLDER_THAN = '1.4.0';
private const WARN_WHEN_EXTENSION_IS_OLDER_THAN = '1.5.0';

/** @var Config */
private $config;
Expand Down Expand Up @@ -320,6 +320,14 @@ function (): ?Span {
foreach ($this->phpExtension->getCalls() as $recordedCall) {
$callSpan = $this->request->startSpan($recordedCall->functionName(), $recordedCall->timeEntered());

$maybeHttpUrl = $recordedCall->maybeHttpUrl();
if ($maybeHttpUrl !== null) {
$httpMethod = $recordedCall->maybeHttpMethod() ?: 'GET';
$httpSpan = $this->request->startSpan('HTTP/' . $httpMethod, $recordedCall->timeEntered());
$httpSpan->tag(Tag::TAG_URI, $maybeHttpUrl);
$this->request->stopSpan($recordedCall->timeExited());
}

$arguments = $recordedCall->filteredArguments();

if (count($arguments) > 0) {
Expand Down Expand Up @@ -448,6 +456,7 @@ public function send(): bool
$this->registerIfRequired();
$this->sendMetadataIfRequired();

$this->addSpansFromExtension();
$this->request->stopIfRunning();

$shouldLogContent = $this->config->get(ConfigKey::LOG_PAYLOAD_CONTENT);
Expand Down
1 change: 1 addition & 0 deletions src/Events/Tag/Tag.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ abstract class Tag implements Command
public const TAG_REQUEST_PATH = 'path';
public const TAG_QUEUE_TIME = 'scout.queue_time_ns';
public const TAG_REACHED_SPAN_CAP = 'scout.reached_span_cap';
public const TAG_URI = 'uri';

/** @var RequestId */
protected $requestId;
Expand Down
87 changes: 85 additions & 2 deletions src/Extension/RecordedCall.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@

use Webmozart\Assert\Assert;

use function array_key_exists;
use function in_array;
use function is_array;
use function is_string;
use function json_decode;
use function preg_replace;
use function stripos;
use function strtoupper;

final class RecordedCall
{
/** @var string */
Expand Down Expand Up @@ -87,16 +96,90 @@ public function timeExited(): float
* avoid potentially spilling personally identifiable information. Another reason to only return specific arguments
* is to avoid sending loads of data unnecessarily.
*
* @return mixed[]
* @return list<empty>|array{url: string, method: string}
*/
public function filteredArguments(): array
{
if ($this->function === 'file_get_contents') {
if ($this->function === 'file_get_contents' || $this->function === 'curl_exec') {
$method = 'GET';

// file_get_contents was used with a stream context
if (
$this->function === 'file_get_contents'
&& array_key_exists(2, $this->arguments)
&& is_string($this->arguments[2])
) {
/** @var mixed $fileGetContentsStreamContext */
$fileGetContentsStreamContext = json_decode($this->arguments[2], true);
if (
is_array($fileGetContentsStreamContext)
&& array_key_exists('http', $fileGetContentsStreamContext)
&& is_array($fileGetContentsStreamContext['http'])
&& array_key_exists('method', $fileGetContentsStreamContext['http'])
&& is_string($fileGetContentsStreamContext['http']['method'])
&& ! empty($fileGetContentsStreamContext['http']['method'])
) {
$method = $fileGetContentsStreamContext['http']['method'];
}
}

// curl_exec with CURLOPT_POST option was used with a truthy value
if ($this->function === 'curl_exec' && array_key_exists(1, $this->arguments) && $this->arguments[1]) {
$method = 'POST';
}

// curl_exec with CURLOPT_POST option was used with a truthy value
if (
$this->function === 'curl_exec'
&& array_key_exists(2, $this->arguments)
&& is_string($this->arguments[2])
&& ! empty($this->arguments[2])
) {
$method = $this->arguments[2];
}

return [
'url' => (string) $this->arguments[0],
'method' => preg_replace('/[^A-Z]/', '', strtoupper($method)),
];
}

return [];
}

public function maybeHttpUrl(): ?string
{
if (! in_array($this->function, ['file_get_contents', 'curl_exec'], true)) {
return null;
}

$arguments = $this->filteredArguments();

if (! array_key_exists('url', $arguments)) {
return null;
}

$url = $arguments['url'];

if (stripos($url, 'http://') !== 0 && stripos($url, 'https://') !== 0) {
return null;
}

return $url;
}

public function maybeHttpMethod(): ?string
{
if (! in_array($this->function, ['file_get_contents', 'curl_exec'], true)) {
return null;
}

$arguments = $this->filteredArguments();

if (! array_key_exists('method', $arguments)) {
return null;
}

return $arguments['method'];
}
}
Loading

0 comments on commit 4e1396f

Please sign in to comment.