cleaning up trace/span id format methods (#1675)

open-telemetry · Mar 8, 2021 · 8aaf3f6 · 8aaf3f6
1 parent a3a5e55
commit 8aaf3f6
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -41,6 +41,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Rename `JaegerSpanExporter` to `JaegerExporter` and rename `ZipkinSpanExporter` to `ZipkinExporter`
   ([#1664](https://github.com/open-telemetry/opentelemetry-python/pull/1664))
 
+### Removed
+- Removed unused `get_hexadecimal_trace_id` and `get_hexadecimal_span_id` methods.
+  ([#1675])(https://github.com/open-telemetry/opentelemetry-python/pull/1675)
+
 ## [0.18b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v0.18b0) - 2021-02-16
 
 ### Added

diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/encoder/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/encoder/__init__.py
@@ -25,7 +25,12 @@
 
 from opentelemetry.exporter.zipkin.node_endpoint import NodeEndpoint
 from opentelemetry.sdk.trace import Event
-from opentelemetry.trace import Span, SpanContext
+from opentelemetry.trace import (
+    Span,
+    SpanContext,
+    format_span_id,
+    format_trace_id,
+)
 from opentelemetry.trace.status import StatusCode
 
 EncodedLocalEndpointT = TypeVar("EncodedLocalEndpointT")
@@ -260,8 +265,8 @@ def _encode_local_endpoint(local_endpoint: NodeEndpoint) -> Dict:
 
     @staticmethod
     def _encode_span_id(span_id: int) -> str:
-        return format(span_id, "016x")
+        return format_span_id(span_id)
 
     @staticmethod
     def _encode_trace_id(trace_id: int) -> str:
-        return format(trace_id, "032x")
+        return format_trace_id(trace_id)
diff --git a/exporter/opentelemetry-exporter-zipkin/tests/encoder/test_v1_json.py b/exporter/opentelemetry-exporter-zipkin/tests/encoder/test_v1_json.py
@@ -18,7 +18,7 @@
 from opentelemetry.exporter.zipkin.encoder.v1.json import JsonV1Encoder
 from opentelemetry.exporter.zipkin.node_endpoint import NodeEndpoint
 from opentelemetry.sdk import trace
-from opentelemetry.trace import TraceFlags
+from opentelemetry.trace import TraceFlags, format_span_id, format_trace_id
 
 from .common_tests import TEST_SERVICE_NAME, CommonEncoderTestCases
 
@@ -194,13 +194,13 @@ def test_encode_id_zero_padding(self):
 
         expected_output = [
             {
-                "traceId": format(trace_id, "032x"),
-                "id": format(span_id, "016x"),
+                "traceId": format_trace_id(trace_id),
+                "id": format_span_id(span_id),
                 "name": TEST_SERVICE_NAME,
                 "timestamp": JsonV1Encoder._nsec_to_usec_round(start_time),
                 "duration": JsonV1Encoder._nsec_to_usec_round(duration),
                 "debug": True,
-                "parentId": format(parent_id, "016x"),
+                "parentId": format_span_id(parent_id),
             }
         ]
 

diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py
@@ -18,6 +18,7 @@
 import opentelemetry.trace as trace
 from opentelemetry.context.context import Context
 from opentelemetry.propagators import textmap
+from opentelemetry.trace import format_span_id, format_trace_id
 from opentelemetry.trace.span import TraceState
 
 
@@ -96,10 +97,10 @@ def inject(
         span_context = span.get_span_context()
         if span_context == trace.INVALID_SPAN_CONTEXT:
             return
-        traceparent_string = "00-{:032x}-{:016x}-{:02x}".format(
-            span_context.trace_id,
-            span_context.span_id,
+        traceparent_string = "00-{trace_id}-{span_id}-{:02x}".format(
             span_context.trace_flags,
+            trace_id=format_trace_id(span_context.trace_id),
+            span_id=format_span_id(span_context.span_id),
         )
         set_in_carrier(
             carrier, self._TRACEPARENT_HEADER_NAME, traceparent_string

diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py
@@ -471,7 +471,7 @@ def __delattr__(self, *args: str) -> None:
 
     def __repr__(self) -> str:
         return (
-            "{}(trace_id={}, span_id={}, trace_flags=0x{:02x}, trace_state={!r}, is_remote={})"
+            "{}(trace_id=0x{}, span_id=0x{}, trace_flags=0x{:02x}, trace_state={!r}, is_remote={})"
         ).format(
             type(self).__name__,
             format_trace_id(self.trace_id),
@@ -548,16 +548,22 @@ def __repr__(self) -> str:
 
 
 def format_trace_id(trace_id: int) -> str:
-    return "0x{:032x}".format(trace_id)
-
-
-def format_span_id(span_id: int) -> str:
-    return "0x{:016x}".format(span_id)
+    """Convenience trace ID formatting method
+    Args:
+        trace_id: Trace ID int
 
+    Returns:
+        The trace ID as 32-byte hexadecimal string
+    """
+    return format(trace_id, "032x")
 
-def get_hexadecimal_trace_id(trace_id: int) -> str:
-    return "{:032x}".format(trace_id)
 
+def format_span_id(span_id: int) -> str:
+    """Convenience span ID formatting method
+    Args:
+        span_id: Span ID int
 
-def get_hexadecimal_span_id(span_id: int) -> str:
-    return "{:016x}".format(span_id)
+    Returns:
+        The span ID as 16-byte hexadecimal string
+    """
+    return format(span_id, "016x")
diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py
@@ -18,6 +18,7 @@
 from opentelemetry.propagate import extract, inject
 from opentelemetry.propagators.textmap import DictGetter
 from opentelemetry.trace import get_current_span, set_span_in_context
+from opentelemetry.trace.span import format_span_id, format_trace_id
 
 carrier_getter = DictGetter()
 
@@ -30,8 +31,8 @@ class TestDefaultGlobalPropagator(unittest.TestCase):
 
     def test_propagation(self):
         traceparent_value = "00-{trace_id}-{span_id}-00".format(
-            trace_id=format(self.TRACE_ID, "032x"),
-            span_id=format(self.SPAN_ID, "016x"),
+            trace_id=format_trace_id(self.TRACE_ID),
+            span_id=format_span_id(self.SPAN_ID),
         )
         tracestate_value = "foo=1,bar=2,baz=3"
         headers = {

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -521,8 +521,12 @@ def to_json(self, indent=4):
     @staticmethod
     def _format_context(context):
         x_ctx = OrderedDict()
-        x_ctx["trace_id"] = trace_api.format_trace_id(context.trace_id)
-        x_ctx["span_id"] = trace_api.format_span_id(context.span_id)
+        x_ctx["trace_id"] = "0x{}".format(
+            trace_api.format_trace_id(context.trace_id)
+        )
+        x_ctx["span_id"] = "0x{}".format(
+            trace_api.format_span_id(context.span_id)
+        )
         x_ctx["trace_state"] = repr(context.trace_state)
         return x_ctx
 

diff --git a/propagator/opentelemetry-propagator-b3/src/opentelemetry/propagators/b3/__init__.py b/propagator/opentelemetry-propagator-b3/src/opentelemetry/propagators/b3/__init__.py
@@ -23,6 +23,7 @@
     TextMapPropagator,
     TextMapPropagatorT,
 )
+from opentelemetry.trace import format_span_id, format_trace_id
 
 
 class B3Format(TextMapPropagator):
@@ -162,16 +163,6 @@ def fields(self) -> typing.Set[str]:
         }
 
 
-def format_trace_id(trace_id: int) -> str:
-    """Format the trace id according to b3 specification."""
-    return format(trace_id, "032x")
-
-
-def format_span_id(span_id: int) -> str:
-    """Format the span id according to b3 specification."""
-    return format(span_id, "016x")
-
-
 def _extract_first_element(
     items: typing.Iterable[TextMapPropagatorT],
 ) -> typing.Optional[TextMapPropagatorT]:

diff --git a/propagator/opentelemetry-propagator-jaeger/src/opentelemetry/propagators/jaeger/__init__.py b/propagator/opentelemetry-propagator-jaeger/src/opentelemetry/propagators/jaeger/__init__.py
@@ -24,6 +24,7 @@
     TextMapPropagator,
     TextMapPropagatorT,
 )
+from opentelemetry.trace import format_span_id, format_trace_id
 
 
 class JaegerPropagator(TextMapPropagator):
@@ -132,8 +133,11 @@ def _extract_baggage(self, getter, carrier, context):
 
 
 def _format_uber_trace_id(trace_id, span_id, parent_span_id, flags):
-    return "{:032x}:{:016x}:{:016x}:{:02x}".format(
-        trace_id, span_id, parent_span_id, flags
+    return "{trace_id}:{span_id}:{parent_id}:{:02x}".format(
+        flags,
+        trace_id=format_trace_id(trace_id),
+        span_id=format_span_id(span_id),
+        parent_id=format_span_id(parent_span_id),
     )
 
 

diff --git a/propagator/opentelemetry-propagator-jaeger/tests/test_jaeger_propagator.py b/propagator/opentelemetry-propagator-jaeger/tests/test_jaeger_propagator.py
@@ -59,20 +59,14 @@ def get_context_new_carrier(old_carrier, carrier_baggage=None):
     return ctx, new_carrier
 
 
-def _format_uber_trace_id(trace_id, span_id, parent_span_id, flags):
-    return "{:032x}:{:016x}:{:016x}:{:02x}".format(
-        trace_id, span_id, parent_span_id, flags
-    )
-
-
 class TestJaegerPropagator(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         generator = id_generator.RandomIdGenerator()
         cls.trace_id = generator.generate_trace_id()
         cls.span_id = generator.generate_span_id()
         cls.parent_span_id = generator.generate_span_id()
-        cls.serialized_uber_trace_id = _format_uber_trace_id(
+        cls.serialized_uber_trace_id = jaeger._format_uber_trace_id(  # pylint: disable=protected-access
             cls.trace_id, cls.span_id, cls.parent_span_id, 11
         )
 
@@ -123,7 +117,7 @@ def test_debug_flag_set(self):
         self.assertEqual(FORMAT.DEBUG_FLAG, debug_flag_value)
 
     def test_sample_debug_flags_unset(self):
-        uber_trace_id = _format_uber_trace_id(
+        uber_trace_id = jaeger._format_uber_trace_id(  # pylint: disable=protected-access
             self.trace_id, self.span_id, self.parent_span_id, 0
         )
         old_carrier = {FORMAT.TRACE_ID_KEY: uber_trace_id}