Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Commit

Permalink
Fix empty start time (#135)
Browse files Browse the repository at this point in the history
* Failing test for default timestamp

* Add phpt tests for extension when you provide invalid startTime options

* Extension ignores invalid startTime options (only allow int/float)

* Extension tracer sets default startTime

* Fix test description
  • Loading branch information
chingor13 authored Mar 5, 2018
1 parent ac4be6d commit 6845ae1
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 5 deletions.
15 changes: 11 additions & 4 deletions ext/opencensus_trace_span.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
zend_class_entry* opencensus_trace_span_ce = NULL;

ZEND_BEGIN_ARG_INFO_EX(arginfo_OpenCensusTraceSpan_construct, 0, 0, 1)
ZEND_ARG_ARRAY_INFO(0, spanOptions, 0)
ZEND_ARG_ARRAY_INFO(0, spanOptions, 0)
ZEND_END_ARG_INFO();

/**
Expand Down Expand Up @@ -468,7 +468,14 @@ int opencensus_trace_span_apply_span_options(opencensus_trace_span_t *span, zval
if (strcmp(ZSTR_VAL(k), "attributes") == 0) {
zend_hash_merge(span->attributes, Z_ARRVAL_P(v), zval_add_ref, 0);
} else if (strcmp(ZSTR_VAL(k), "startTime") == 0) {
span->start = Z_DVAL_P(v);
switch (Z_TYPE_P(v)) {
case IS_DOUBLE:
span->start = Z_DVAL_P(v);
break;
case IS_LONG:
span->start = (double) Z_LVAL_P(v);
break;
}
} else if (strcmp(ZSTR_VAL(k), "name") == 0) {
if (span->name) {
zend_string_release(span->name);
Expand Down Expand Up @@ -504,7 +511,7 @@ static int opencensus_trace_update_time_events(opencensus_trace_span_t *span, zv
opencensus_trace_time_event_to_zval(event, &zv);
add_next_index_zval(return_value, &zv);
} ZEND_HASH_FOREACH_END();
return SUCCESS;
return SUCCESS;
}

static int opencensus_trace_update_links(opencensus_trace_span_t *span, zval *return_value)
Expand All @@ -515,7 +522,7 @@ static int opencensus_trace_update_links(opencensus_trace_span_t *span, zval *re
opencensus_trace_link_to_zval(link, &zv);
add_next_index_zval(return_value, &zv);
} ZEND_HASH_FOREACH_END();
return SUCCESS;
return SUCCESS;
}

/* Fill the provided span with the provided data from the internal span representation */
Expand Down
21 changes: 21 additions & 0 deletions ext/tests/int_start_time.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
OpenCensus Trace: Bug 131: Providing integer start time should work
--FILE--
<?php

opencensus_trace_begin('foo', ['startTime' => (int) microtime(true)]);
opencensus_trace_finish();
$spans = opencensus_trace_list();

echo 'Number of spans: ' . count($spans) . PHP_EOL;
$span = $spans[0];
var_dump($span->startTime());

$test = microtime(true) - 1 < $span->startTime();
echo 'Start time just happened: ' . $test;

?>
--EXPECTF--
Number of spans: 1
float(%d)
Start time just happened: 1
21 changes: 21 additions & 0 deletions ext/tests/null_start_time.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
OpenCensus Trace: Bug 131: Providing null start time defaults to a current time
--FILE--
<?php

opencensus_trace_begin('foo', ['startTime' => null]);
opencensus_trace_finish();
$spans = opencensus_trace_list();

echo 'Number of spans: ' . count($spans) . PHP_EOL;
$span = $spans[0];
var_dump($span->startTime());

$test = microtime(true) - 1 < $span->startTime();
echo 'Start time just happened: ' . $test;

?>
--EXPECTF--
Number of spans: 1
float(%d.%d)
Start time just happened: 1
2 changes: 1 addition & 1 deletion src/Trace/Tracer/ExtensionTracer.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public function withSpan(Span $span)
{
$startTime = $span->startTime()
? (float)($span->startTime()->format('U.u'))
: null;
: microtime(true);
$info = [
'spanId' => $span->spanId(),
'parentSpanId' => $span->parentSpanId(),
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/Trace/Tracer/AbstractTracerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,22 @@ public function testAddsMessageEventToRootSpan()
$this->assertCount(1, $span->timeEvents());
}

public function testInSpanSetsDefaultStartTime()
{
$class = $this->getTracerClass();
$tracer = new $class();
$tracer->inSpan(['name' => 'foo'], function () {
// do nothing
});

$spans = $tracer->spans();
$this->assertCount(1, $spans);
$span = $spans[0];

// #131 - Span should be initialized with current time, not the epoch.
$this->assertNotEquals(0, $span->startTime()->getTimestamp());
}

private function assertEquivalentTimestamps($expected, $value)
{
$this->assertEquals((float)($expected->format('U.u')), (float)($expected->format('U.u')), '', 0.000001);
Expand Down

0 comments on commit 6845ae1

Please sign in to comment.