Skip to content

Commit

Permalink
OTLP exporter is encoding invalid span/trace IDs in the logs fix (#4006)
Browse files Browse the repository at this point in the history
  • Loading branch information
soumyadeepm04 authored Jul 11, 2024
1 parent 61ea97d commit ee1b008
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- OTLP exporter is encoding invalid span/trace IDs in the logs fix
([#4006](https://github.com/open-telemetry/opentelemetry-python/pull/4006))
- Update sdk process resource detector `process.command_args` attribute to also include the executable itself
([#4032](https://github.com/open-telemetry/opentelemetry-python/pull/4032))
- Fix `start_time_unix_nano` for delta collection for explicit bucket histogram aggregation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,21 @@ def encode_logs(batch: Sequence[LogData]) -> ExportLogsServiceRequest:


def _encode_log(log_data: LogData) -> PB2LogRecord:
span_id = (
None
if log_data.log_record.span_id == 0
else _encode_span_id(log_data.log_record.span_id)
)
trace_id = (
None
if log_data.log_record.trace_id == 0
else _encode_trace_id(log_data.log_record.trace_id)
)
return PB2LogRecord(
time_unix_nano=log_data.log_record.timestamp,
observed_time_unix_nano=log_data.log_record.observed_timestamp,
span_id=_encode_span_id(log_data.log_record.span_id),
trace_id=_encode_trace_id(log_data.log_record.trace_id),
span_id=span_id,
trace_id=trace_id,
flags=int(log_data.log_record.trace_flags),
body=_encode_value(log_data.log_record.body),
severity_text=log_data.log_record.severity_text,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,8 @@ def get_test_logs(
PB2LogRecord(
time_unix_nano=1644650249738562048,
observed_time_unix_nano=1644650249738562049,
trace_id=_encode_trace_id(0),
span_id=_encode_span_id(0),
trace_id=None,
span_id=None,
flags=int(TraceFlags.DEFAULT),
severity_text="WARN",
severity_number=SeverityNumber.WARN.value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from google.protobuf.duration_pb2 import ( # pylint: disable=no-name-in-module
Duration,
)
from google.protobuf.json_format import MessageToDict
from google.rpc.error_details_pb2 import RetryInfo
from grpc import ChannelCredentials, Compression, StatusCode, server

Expand Down Expand Up @@ -167,6 +168,36 @@ def setUp(self):
"third_name", "third_version"
),
)
self.log_data_4 = LogData(
log_record=LogRecord(
timestamp=int(time.time() * 1e9),
trace_id=0,
span_id=5213367945872657629,
trace_flags=TraceFlags(0x01),
severity_text="ERROR",
severity_number=SeverityNumber.WARN,
body="Invalid trace id check",
resource=SDKResource({"service": "myapp"}),
),
instrumentation_scope=InstrumentationScope(
"fourth_name", "fourth_version"
),
)
self.log_data_5 = LogData(
log_record=LogRecord(
timestamp=int(time.time() * 1e9),
trace_id=2604504634922341076776623263868986801,
span_id=0,
trace_flags=TraceFlags(0x01),
severity_text="ERROR",
severity_number=SeverityNumber.WARN,
body="Invalid span id check",
resource=SDKResource({"service": "myapp"}),
),
instrumentation_scope=InstrumentationScope(
"fifth_name", "fifth_version"
),
)

def tearDown(self):
self.server.stop(None)
Expand Down Expand Up @@ -342,6 +373,43 @@ def test_failure(self):
self.exporter.export([self.log_data_1]), LogExportResult.FAILURE
)

def export_log_and_deserialize(self, log_data):
# pylint: disable=protected-access
translated_data = self.exporter._translate_data([log_data])
request_dict = MessageToDict(translated_data)
log_records = (
request_dict.get("resourceLogs")[0]
.get("scopeLogs")[0]
.get("logRecords")
)
return log_records

def test_exported_log_without_trace_id(self):
log_records = self.export_log_and_deserialize(self.log_data_4)
if log_records:
log_record = log_records[0]
self.assertIn("spanId", log_record)
self.assertNotIn(
"traceId",
log_record,
"traceId should not be present in the log record",
)
else:
self.fail("No log records found")

def test_exported_log_without_span_id(self):
log_records = self.export_log_and_deserialize(self.log_data_5)
if log_records:
log_record = log_records[0]
self.assertIn("traceId", log_record)
self.assertNotIn(
"spanId",
log_record,
"spanId should not be present in the log record",
)
else:
self.fail("No log records found")

def test_translate_log_data(self):

expected = ExportLogsServiceRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import requests
import responses
from google.protobuf.json_format import MessageToDict

from opentelemetry._logs import SeverityNumber
from opentelemetry.exporter.otlp.proto.http import Compression
Expand All @@ -31,6 +32,9 @@
OTLPLogExporter,
)
from opentelemetry.exporter.otlp.proto.http.version import __version__
from opentelemetry.proto.collector.logs.v1.logs_service_pb2 import (
ExportLogsServiceRequest,
)
from opentelemetry.sdk._logs import LogData
from opentelemetry.sdk._logs import LogRecord as SDKLogRecord
from opentelemetry.sdk._logs.export import LogExportResult
Expand Down Expand Up @@ -167,6 +171,76 @@ def test_exporter_env(self):
)
self.assertIsInstance(exporter._session, requests.Session)

@staticmethod
def export_log_and_deserialize(log):
with patch("requests.Session.post") as mock_post:
exporter = OTLPLogExporter()
exporter.export([log])
request_body = mock_post.call_args[1]["data"]
request = ExportLogsServiceRequest()
request.ParseFromString(request_body)
request_dict = MessageToDict(request)
log_records = (
request_dict.get("resourceLogs")[0]
.get("scopeLogs")[0]
.get("logRecords")
)
return log_records

def test_exported_log_without_trace_id(self):
log = LogData(
log_record=SDKLogRecord(
timestamp=1644650195189786182,
trace_id=0,
span_id=1312458408527513292,
trace_flags=TraceFlags(0x01),
severity_text="WARN",
severity_number=SeverityNumber.WARN,
body="Invalid trace id check",
resource=SDKResource({"first_resource": "value"}),
attributes={"a": 1, "b": "c"},
),
instrumentation_scope=InstrumentationScope("name", "version"),
)
log_records = TestOTLPHTTPLogExporter.export_log_and_deserialize(log)
if log_records:
log_record = log_records[0]
self.assertIn("spanId", log_record)
self.assertNotIn(
"traceId",
log_record,
"trace_id should not be present in the log record",
)
else:
self.fail("No log records found")

def test_exported_log_without_span_id(self):
log = LogData(
log_record=SDKLogRecord(
timestamp=1644650195189786360,
trace_id=89564621134313219400156819398935297696,
span_id=0,
trace_flags=TraceFlags(0x01),
severity_text="WARN",
severity_number=SeverityNumber.WARN,
body="Invalid span id check",
resource=SDKResource({"first_resource": "value"}),
attributes={"a": 1, "b": "c"},
),
instrumentation_scope=InstrumentationScope("name", "version"),
)
log_records = TestOTLPHTTPLogExporter.export_log_and_deserialize(log)
if log_records:
log_record = log_records[0]
self.assertIn("traceId", log_record)
self.assertNotIn(
"spanId",
log_record,
"spanId should not be present in the log record",
)
else:
self.fail("No log records found")

@responses.activate
@patch("opentelemetry.exporter.otlp.proto.http._log_exporter.sleep")
def test_exponential_backoff(self, mock_sleep):
Expand Down

0 comments on commit ee1b008

Please sign in to comment.