Skip to content

Commit 033bcdc

Browse files
alrexc24t
alrex
authored and
c24t
committed
Remove create_span from the API (open-telemetry#290)
Simplify the API by removing create_span. Resolves open-telemetry#152
1 parent 693391c commit 033bcdc

File tree

10 files changed

+51
-135
lines changed

10 files changed

+51
-135
lines changed

‎ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,12 @@ def _before_flask_request():
6262

6363
tracer = trace.tracer()
6464

65-
span = tracer.create_span(
66-
span_name, parent_span, kind=trace.SpanKind.SERVER
65+
span = tracer.start_span(
66+
span_name,
67+
parent_span,
68+
kind=trace.SpanKind.SERVER,
69+
start_time=environ.get(_ENVIRON_STARTTIME_KEY),
6770
)
68-
span.start(environ.get(_ENVIRON_STARTTIME_KEY))
6971
activation = tracer.use_span(span, end_on_exit=True)
7072
activation.__enter__()
7173
environ[_ENVIRON_ACTIVATION_KEY] = activation

‎ext/opentelemetry-ext-flask/tests/test_flask_integration.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# limitations under the License.
1414

1515
import unittest
16+
from unittest import mock
1617

1718
from flask import Flask
1819
from werkzeug.test import Client
@@ -52,12 +53,12 @@ def test_simple(self):
5253
self.assertEqual(200, resp.status_code)
5354
self.assertEqual([b"Hello: 123"], list(resp.response))
5455

55-
self.create_span.assert_called_with(
56+
self.start_span.assert_called_with(
5657
"hello_endpoint",
5758
trace_api.INVALID_SPAN_CONTEXT,
5859
kind=trace_api.SpanKind.SERVER,
60+
start_time=mock.ANY,
5961
)
60-
self.assertEqual(1, self.span.start.call_count)
6162

6263
# TODO: Change this test to use the SDK, as mocking becomes painful
6364

@@ -79,12 +80,12 @@ def test_404(self):
7980
self.assertEqual(404, resp.status_code)
8081
resp.close()
8182

82-
self.create_span.assert_called_with(
83+
self.start_span.assert_called_with(
8384
"/bye",
8485
trace_api.INVALID_SPAN_CONTEXT,
8586
kind=trace_api.SpanKind.SERVER,
87+
start_time=mock.ANY,
8688
)
87-
self.assertEqual(1, self.span.start.call_count)
8889

8990
# Nope, this uses Tracer.use_span(end_on_exit)
9091
# self.assertEqual(1, self.span.end.call_count)
@@ -107,12 +108,12 @@ def test_internal_error(self):
107108
self.assertEqual(500, resp.status_code)
108109
resp.close()
109110

110-
self.create_span.assert_called_with(
111+
self.start_span.assert_called_with(
111112
"hello_endpoint",
112113
trace_api.INVALID_SPAN_CONTEXT,
113114
kind=trace_api.SpanKind.SERVER,
115+
start_time=mock.ANY,
114116
)
115-
self.assertEqual(1, self.span.start.call_count)
116117

117118
# Nope, this uses Tracer.use_span(end_on_exit)
118119
# self.assertEqual(1, self.span.end.call_count)

‎ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -238,22 +238,21 @@ def start_span(
238238
for ref in references:
239239
links.append(trace_api.Link(ref.referenced_context.unwrap()))
240240

241-
span = self._otel_tracer.create_span(
242-
operation_name, parent, links=links
243-
)
244-
245-
if tags:
246-
for key, value in tags.items():
247-
span.set_attribute(key, value)
248-
249241
# The OpenTracing API expects time values to be `float` values which
250242
# represent the number of seconds since the epoch. OpenTelemetry
251243
# represents time values as nanoseconds since the epoch.
252244
start_time_ns = start_time
253245
if start_time_ns is not None:
254246
start_time_ns = util.time_seconds_to_ns(start_time)
255247

256-
span.start(start_time=start_time_ns)
248+
span = self._otel_tracer.start_span(
249+
operation_name,
250+
parent,
251+
links=links,
252+
attributes=tags,
253+
start_time=start_time_ns,
254+
)
255+
257256
context = SpanContextShim(span.get_context())
258257
return SpanShim(self, context, span)
259258

‎ext/opentelemetry-ext-testutil/src/opentelemetry/ext/testutil/wsgitestutil.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@ class WsgiTestBase(unittest.TestCase):
1010
def setUp(self):
1111
tracer = trace_api.tracer()
1212
self.span = mock.create_autospec(trace_api.Span, spec_set=True)
13-
self.create_span_patcher = mock.patch.object(
13+
self.start_span_patcher = mock.patch.object(
1414
tracer,
15-
"create_span",
15+
"start_span",
1616
autospec=True,
1717
spec_set=True,
1818
return_value=self.span,
1919
)
20-
self.create_span = self.create_span_patcher.start()
20+
self.start_span = self.start_span_patcher.start()
2121
self.write_buffer = io.BytesIO()
2222
self.write = self.write_buffer.write
2323

@@ -29,12 +29,9 @@ def setUp(self):
2929
self.exc_info = None
3030

3131
def tearDown(self):
32-
self.create_span_patcher.stop()
32+
self.start_span_patcher.stop()
3333

3434
def start_response(self, status, response_headers, exc_info=None):
35-
# The span should have started already
36-
self.assertEqual(1, self.span.start.call_count)
37-
3835
self.status = status
3936
self.response_headers = response_headers
4037
self.exc_info = exc_info

‎ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424

2525
from opentelemetry import propagators, trace
2626
from opentelemetry.ext.wsgi.version import __version__ # noqa
27-
from opentelemetry.util import time_ns
2827

2928

3029
def get_header_from_environ(
@@ -138,16 +137,13 @@ def __call__(self, environ, start_response):
138137
start_response: The WSGI start_response callable.
139138
"""
140139

141-
start_timestamp = time_ns()
142-
143140
tracer = trace.tracer()
144141
parent_span = propagators.extract(get_header_from_environ, environ)
145142
span_name = get_default_span_name(environ)
146143

147-
span = tracer.create_span(
144+
span = tracer.start_span(
148145
span_name, parent_span, kind=trace.SpanKind.SERVER
149146
)
150-
span.start(start_timestamp)
151147

152148
try:
153149
with tracer.use_span(span):

‎ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,9 @@ def validate_response(self, response, error=None):
9696
self.assertIsNone(self.exc_info)
9797

9898
# Verify that start_span has been called
99-
self.create_span.assert_called_with(
99+
self.start_span.assert_called_with(
100100
"/", trace_api.INVALID_SPAN_CONTEXT, kind=trace_api.SpanKind.SERVER
101101
)
102-
self.assertEqual(1, self.span.start.call_count)
103102

104103
def test_basic_wsgi_call(self):
105104
app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi)

‎opentelemetry-api/src/opentelemetry/trace/__init__.py

Lines changed: 8 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@
3030
created as children of the currently active span, and the newly-created span
3131
can optionally become the new active span::
3232
33-
from opentelemetry.trace import tracer
33+
from opentelemetry import trace
3434
3535
# Create a new root span, set it as the current span in context
36-
with tracer.start_as_current_span("parent"):
36+
with trace.tracer().start_as_current_span("parent"):
3737
# Attach a new child and update the current span
3838
with tracer.start_as_current_span("child"):
3939
do_work():
@@ -43,17 +43,15 @@
4343
When creating a span that's "detached" from the context the active span doesn't
4444
change, and the caller is responsible for managing the span's lifetime::
4545
46-
from opentelemetry.api.trace import tracer
46+
from opentelemetry import trace
4747
4848
# Explicit parent span assignment
49-
span = tracer.create_span("child", parent=parent) as child:
49+
child = trace.tracer().start_span("child", parent=parent)
5050
51-
# The caller is responsible for starting and ending the span
52-
span.start()
5351
try:
5452
do_work(span=child)
5553
finally:
56-
span.end()
54+
child.end()
5755
5856
Applications should generally use a single global tracer, and use either
5957
implicit or explicit context propagation consistently throughout.
@@ -147,16 +145,6 @@ class SpanKind(enum.Enum):
147145
class Span:
148146
"""A span represents a single operation within a trace."""
149147

150-
def start(self, start_time: typing.Optional[int] = None) -> None:
151-
"""Sets the current time as the span's start time.
152-
153-
Each span represents a single operation. The span's start time is the
154-
wall time at which the operation started.
155-
156-
Only the first call to `start` should modify the span, and
157-
implementations are free to ignore or raise on further calls.
158-
"""
159-
160148
def end(self, end_time: int = None) -> None:
161149
"""Sets the current time as the span's end time.
162150
@@ -204,8 +192,7 @@ def add_lazy_event(self, event: Event) -> None:
204192
def update_name(self, name: str) -> None:
205193
"""Updates the `Span` name.
206194
207-
This will override the name provided via :func:`Tracer.create_span`
208-
or :func:`Tracer.start_span`.
195+
This will override the name provided via :func:`Tracer.start_span`.
209196
210197
Upon this update, any sampling behavior based on Span name will depend
211198
on the implementation.
@@ -404,6 +391,7 @@ def start_span(
404391
kind: SpanKind = SpanKind.INTERNAL,
405392
attributes: typing.Optional[types.Attributes] = None,
406393
links: typing.Sequence[Link] = (),
394+
start_time: typing.Optional[int] = None,
407395
) -> "Span":
408396
"""Starts a span.
409397
@@ -434,6 +422,7 @@ def start_span(
434422
meaningful even if there is no parent.
435423
attributes: The span's attributes.
436424
links: Links span to other spans
425+
start_time: Sets the start time of a span
437426
438427
Returns:
439428
The newly-created span.
@@ -494,51 +483,6 @@ def start_as_current_span(
494483
# pylint: disable=unused-argument,no-self-use
495484
yield INVALID_SPAN
496485

497-
def create_span(
498-
self,
499-
name: str,
500-
parent: ParentSpan = CURRENT_SPAN,
501-
kind: SpanKind = SpanKind.INTERNAL,
502-
attributes: typing.Optional[types.Attributes] = None,
503-
links: typing.Sequence[Link] = (),
504-
) -> "Span":
505-
"""Creates a span.
506-
507-
Creating the span does not start it, and should not affect the tracer's
508-
context. To start the span and update the tracer's context to make it
509-
the currently active span, see :meth:`use_span`.
510-
511-
By default the current span will be used as parent, but an explicit
512-
parent can also be specified, either a Span or a SpanContext.
513-
If the specified value is `None`, the created span will be a root
514-
span.
515-
516-
Applications that need to create spans detached from the tracer's
517-
context should use this method.
518-
519-
with tracer.start_as_current_span(name) as span:
520-
do_work()
521-
522-
This is equivalent to::
523-
524-
span = tracer.create_span(name)
525-
with tracer.use_span(span):
526-
do_work()
527-
528-
Args:
529-
name: The name of the span to be created.
530-
parent: The span's parent. Defaults to the current span.
531-
kind: The span's kind (relationship to parent). Note that is
532-
meaningful even if there is no parent.
533-
attributes: The span's attributes.
534-
links: Links span to other spans
535-
536-
Returns:
537-
The newly-created span.
538-
"""
539-
# pylint: disable=unused-argument,no-self-use
540-
return INVALID_SPAN
541-
542486
@contextmanager # type: ignore
543487
def use_span(
544488
self, span: "Span", end_on_exit: bool = False

‎opentelemetry-api/tests/trace/test_tracer.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ def test_start_as_current_span(self):
3333
with self.tracer.start_as_current_span("") as span:
3434
self.assertIsInstance(span, trace.Span)
3535

36-
def test_create_span(self):
37-
span = self.tracer.create_span("")
38-
self.assertIsInstance(span, trace.Span)
39-
4036
def test_use_span(self):
4137
span = trace.Span()
4238
with self.tracer.use_span(span):

‎opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -314,20 +314,6 @@ def get_current_span(self):
314314
"""See `opentelemetry.trace.Tracer.get_current_span`."""
315315
return self._current_span_slot.get()
316316

317-
def start_span(
318-
self,
319-
name: str,
320-
parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN,
321-
kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
322-
attributes: Optional[types.Attributes] = None,
323-
links: Sequence[trace_api.Link] = (),
324-
) -> "Span":
325-
"""See `opentelemetry.trace.Tracer.start_span`."""
326-
327-
span = self.create_span(name, parent, kind, attributes, links)
328-
span.start()
329-
return span
330-
331317
def start_as_current_span(
332318
self,
333319
name: str,
@@ -341,22 +327,16 @@ def start_as_current_span(
341327
span = self.start_span(name, parent, kind, attributes, links)
342328
return self.use_span(span, end_on_exit=True)
343329

344-
def create_span(
330+
def start_span(
345331
self,
346332
name: str,
347333
parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN,
348334
kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
349335
attributes: Optional[types.Attributes] = None,
350336
links: Sequence[trace_api.Link] = (),
351-
) -> "trace_api.Span":
352-
"""See `opentelemetry.trace.Tracer.create_span`.
353-
354-
If `parent` is null the new span will be created as a root span, i.e. a
355-
span with no parent context. By default, the new span will be created
356-
as a child of the current span in this tracer's context, or as a root
357-
span if no current span exists.
358-
"""
359-
span_id = generate_span_id()
337+
start_time: Optional[int] = None,
338+
) -> "Span":
339+
"""See `opentelemetry.trace.Tracer.start_span`."""
360340

361341
if parent is Tracer.CURRENT_SPAN:
362342
parent = self.get_current_span()
@@ -381,7 +361,7 @@ def create_span(
381361
trace_state = parent_context.trace_state
382362

383363
context = trace_api.SpanContext(
384-
trace_id, span_id, trace_options, trace_state
364+
trace_id, generate_span_id(), trace_options, trace_state
385365
)
386366

387367
# The sampler decides whether to create a real or no-op span at the
@@ -405,7 +385,7 @@ def create_span(
405385
# apply sampling decision attributes after initial attributes
406386
span_attributes = attributes.copy()
407387
span_attributes.update(sampling_decision.attributes)
408-
return Span(
388+
span = Span(
409389
name=name,
410390
context=context,
411391
parent=parent,
@@ -415,8 +395,10 @@ def create_span(
415395
kind=kind,
416396
links=links,
417397
)
418-
419-
return trace_api.DefaultSpan(context=context)
398+
span.start(start_time=start_time)
399+
else:
400+
span = trace_api.DefaultSpan(context=context)
401+
return span
420402

421403
@contextmanager
422404
def use_span(

0 commit comments

Comments
 (0)