Skip to content

Commit

Permalink
fixing warnings from local build (#1277)
Browse files Browse the repository at this point in the history
A couple of warnings have popped up recently:
- docker compose 'version' is deprecated
- BSD host id test was running a command that doesn't exist, which was visible in test output (removed test case)
- synchronous metric stream test was triggering a warnings which was visible in test output (changed to use LogsMessagesTrait)
- resource merging with different schema URLs was triggering a warnings from Config test (updated yaml schema_url and documented)
  • Loading branch information
brettmc authored Apr 18, 2024
1 parent 4333929 commit 5019361
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 13 deletions.
1 change: 0 additions & 1 deletion docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
version: '3.7'
services:
php:
image: ghcr.io/open-telemetry/opentelemetry-php/opentelemetry-php-base:${PHP_VERSION:-8.1}
Expand Down
5 changes: 5 additions & 0 deletions script/semantic-conventions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,8 @@ diff <(grep "public const" src/SemConv/TraceAttributes.php | sort -u) \
```

Use this output as a basis for updating the relevant deprecations file and generate a second time to include them in the final output.

## Update tests

Update `tests/Integration/Config/configurations/kitchen-sink.yaml`'s `resource.schema_url` value to the latest, as merging resources
with different schema URLs is a merging error, per spec.
7 changes: 4 additions & 3 deletions src/SDK/Metrics/Stream/SynchronousMetricStream.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,26 @@
namespace OpenTelemetry\SDK\Metrics\Stream;

use function assert;
use const E_USER_WARNING;
use function extension_loaded;
use GMP;
use function gmp_init;
use function is_int;
use OpenTelemetry\API\Behavior\LogsMessagesTrait;
use OpenTelemetry\SDK\Metrics\AggregationInterface;
use OpenTelemetry\SDK\Metrics\Data\DataInterface;
use OpenTelemetry\SDK\Metrics\Data\Exemplar;
use OpenTelemetry\SDK\Metrics\Data\Temporality;
use const PHP_INT_SIZE;
use function sprintf;
use function trigger_error;

/**
* @internal
* @phan-file-suppress PhanUndeclaredTypeParameter, PhanUndeclaredTypeProperty
*/
final class SynchronousMetricStream implements MetricStreamInterface
{
use LogsMessagesTrait;

private readonly DeltaStorage $delta;
private int|GMP $readers = 0;
private int|GMP $cumulative = 0;
Expand Down Expand Up @@ -62,7 +63,7 @@ public function register($temporality): int

if ($reader === (PHP_INT_SIZE << 3) - 1 && is_int($this->readers)) {
if (!extension_loaded('gmp')) {
trigger_error(sprintf('GMP extension required to register over %d readers', (PHP_INT_SIZE << 3) - 1), E_USER_WARNING);
self::logWarning(sprintf('GMP extension required to register over %d readers', (PHP_INT_SIZE << 3) - 1));
$reader = PHP_INT_SIZE << 3;
} else {
assert(is_int($this->cumulative));
Expand Down
6 changes: 3 additions & 3 deletions src/SDK/Resource/Detectors/Host.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ private function getBsdId(): ?string

$out = exec('kenv -q smbios.system.uuid');

if ($out != false) {
if ($out !== false) {
return $out;
}

Expand All @@ -79,7 +79,7 @@ private function getMacOsId(): ?string
{
$out = exec('ioreg -rd1 -c IOPlatformExpertDevice | awk \'/IOPlatformUUID/ { split($0, line, "\""); printf("%s\n", line[4]); }\'');

if ($out != false) {
if ($out !== false) {
return $out;
}

Expand All @@ -90,7 +90,7 @@ private function getWindowsId(): ?string
{
$out = exec('powershell.exe -Command "Get-ItemPropertyValue -Path HKLM:\SOFTWARE\Microsoft\Cryptography -Name MachineGuid"');

if ($out != false) {
if ($out !== false) {
return $out;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/Integration/Config/configurations/kitchen-sink.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -349,4 +349,4 @@ resource:
# Environment variable: OTEL_SERVICE_NAME
service.name: !!str "unknown_service"
# Configure the resource schema URL.
schema_url: https://opentelemetry.io/schemas/1.23.1
schema_url: https://opentelemetry.io/schemas/1.25.0
9 changes: 4 additions & 5 deletions tests/Unit/SDK/Resource/Detectors/HostTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ class HostTest extends TestCase
{
public function test_host_get_resource(): void
{
$resouceDetector = new Detectors\Host();
$resource = $resouceDetector->getResource();
$resourceDetector = new Detectors\Host();
$resource = $resourceDetector->getResource();

$this->assertSame(ResourceAttributes::SCHEMA_URL, $resource->getSchemaUrl());
$this->assertIsString($resource->getAttributes()->get(ResourceAttributes::HOST_NAME));
Expand All @@ -30,8 +30,8 @@ public function test_host_get_resource(): void
public function test_host_id_filesystem(string $os, array $files, ?string $expectedId): void
{
$root = vfsStream::setup('/', null, $files);
$resouceDetector = new Detectors\Host($root->url(), $os);
$resource = $resouceDetector->getResource();
$resourceDetector = new Detectors\Host($root->url(), $os);
$resource = $resourceDetector->getResource();

if ($expectedId === null) {
$this->assertFalse($resource->getAttributes()->has(ResourceAttributes::HOST_ID));
Expand Down Expand Up @@ -71,7 +71,6 @@ public static function hostIdData(): array
['Linux', $etc_machineid, '1234567890'],
['Linux', array_merge($etc_machineid, $varLibDbus), '1234567890'],
['Linux', $etc_machineid, '1234567890'],
['BSD', [], null],
['BSD', $etc_hostid, '1234567890'],
];
}
Expand Down

0 comments on commit 5019361

Please sign in to comment.