Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OTLP exporter is encoding invalid span/trace IDs in the logs fix #4006

Merged
merged 23 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e210369
OTLP exporter encodes invalid trace/span IDs fix
soumyadeepm04 Jun 28, 2024
3647d61
fix test cases and lint issues
soumyadeepm04 Jun 28, 2024
0b66b33
Merge branch 'main' into newbranch
xrmx Jul 1, 2024
6dc18af
updated changelog
soumyadeepm04 Jul 1, 2024
1496551
update changelog
soumyadeepm04 Jul 1, 2024
bae9d33
Merge branch 'main' into newbranch
lzchen Jul 2, 2024
36d3fd6
Merge branch 'main' into newbranch
lzchen Jul 2, 2024
e2d0d42
Merge branch 'main' into newbranch
soumyadeepm04 Jul 2, 2024
0a2af9b
Merge branch 'main' into newbranch
soumyadeepm04 Jul 2, 2024
be59ef7
initial commit
soumyadeepm04 Jul 3, 2024
3fbd9a9
initial commit
soumyadeepm04 Jul 3, 2024
2eb33a7
added test cases
soumyadeepm04 Jul 3, 2024
49e31e7
resolve merge conflict
soumyadeepm04 Jul 3, 2024
9a13b80
Merge branch 'newbranch' of https://github.com/soumyadeepm04/opentele…
soumyadeepm04 Jul 3, 2024
ba6b263
fix linting issues
soumyadeepm04 Jul 3, 2024
65cd351
fixing lint issues
soumyadeepm04 Jul 3, 2024
d35d0f1
Merge branch 'main' into newbranch
soumyadeepm04 Jul 5, 2024
563569f
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
soumyadeepm04 Jul 8, 2024
4cbefa7
refactored tests
soumyadeepm04 Jul 8, 2024
3f938de
Merge branch 'newbranch' of https://github.com/soumyadeepm04/opentele…
soumyadeepm04 Jul 8, 2024
7f6b95c
fixed lint issue
soumyadeepm04 Jul 8, 2024
92be124
Merge branch 'main' into newbranch
soumyadeepm04 Jul 10, 2024
8b9f90f
Merge branch 'main' into newbranch
lzchen Jul 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
lzchen marked this conversation as resolved.
Show resolved Hide resolved
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