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

Update the body type in the log #3343

Merged
merged 11 commits into from
Jul 12, 2023
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased


- Update the body type in the log
([$3343](https://github.com/open-telemetry/opentelemetry-python/pull/3343))
- Add max_scale option to Exponential Bucket Histogram Aggregation
([#3323](https://github.com/open-telemetry/opentelemetry-python/pull/3323))
- Use BoundedAttributes instead of raw dict to extract attributes from LogRecord
Expand All @@ -23,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed bug where logging export is tracked as trace
[#3375](https://github.com/open-telemetry/opentelemetry-python/pull/3375))


## Version 1.18.0/0.39b0 (2023-05-19)

- Select histogram aggregation with an environment variable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import traceback
from os import environ
from time import time_ns
from typing import Any, Callable, Optional, Tuple, Union
from typing import Any, Callable, Optional, Tuple, Union # noqa

from opentelemetry._logs import Logger as APILogger
from opentelemetry._logs import LoggerProvider as APILoggerProvider
Expand Down Expand Up @@ -476,15 +476,56 @@ def _translate(self, record: logging.LogRecord) -> LogRecord:
timestamp = int(record.created * 1e9)
span_context = get_current_span().get_span_context()
attributes = self._get_attributes(record)
# This comment is taken from GanyedeNil's PR #3343, I have redacted it
# slightly for clarity:
# According to the definition of the Body field type in the
# OTel 1.22.0 Logs Data Model article, the Body field should be of
# type 'any' and should not use the str method to directly translate
# the msg. This is because str only converts non-text types into a
# human-readable form, rather than a standard format, which leads to
# the need for additional operations when collected through a log
# collector.
# Considering that he Body field should be of type 'any' and should not
# use the str method but record.msg is also a string type, then the
# difference is just the self.args formatting?
# The primary consideration depends on the ultimate purpose of the log.
# Converting the default log directly into a string is acceptable as it
# will be required to be presented in a more readable format. However,
# this approach might not be as "standard" when hoping to aggregate
# logs and perform subsequent data analysis. In the context of log
# extraction, it would be more appropriate for the msg to be
# converted into JSON format or remain unchanged, as it will eventually
# be transformed into JSON. If the final output JSON data contains a
# structure that appears similar to JSON but is not, it may confuse
# users. This is particularly true for operation and maintenance
# personnel who need to deal with log data in various languages.
# Where is the JSON converting occur? and what about when the msg
# represents something else but JSON, the expected behavior change?
# For the ConsoleLogExporter, it performs the to_json operation in
# opentelemetry.sdk._logs._internal.export.ConsoleLogExporter.__init__,
# so it can handle any type of input without problems. As for the
# OTLPLogExporter, it also handles any type of input encoding in
# _encode_log located in
# opentelemetry.exporter.otlp.proto.common._internal._log_encoder.
# Therefore, no extra operation is needed to support this change.
# The only thing to consider is the users who have already been using
# this SDK. If they upgrade the SDK after this change, they will need
# to readjust their logging collection rules to adapt to the latest
# output format. Therefore, this change is considered a breaking
# change and needs to be upgraded at an appropriate time.
severity_number = std_to_otel(record.levelno)
if isinstance(record.msg, str) and record.args:
body = record.msg % record.args
else:
body = record.msg
return LogRecord(
timestamp=timestamp,
trace_id=span_context.trace_id,
span_id=span_context.span_id,
trace_flags=span_context.trace_flags,
severity_text=record.levelname,
severity_number=severity_number,
body=record.getMessage(),
body=body,
resource=self._logger.resource,
attributes=attributes,
)
Expand Down
34 changes: 34 additions & 0 deletions opentelemetry-sdk/tests/logs/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,40 @@ def test_simple_log_record_processor_shutdown(self):
finished_logs = exporter.get_finished_logs()
self.assertEqual(len(finished_logs), 0)

def test_simple_log_record_processor_different_msg_types(self):
exporter = InMemoryLogExporter()
log_record_processor = BatchLogRecordProcessor(exporter)

provider = LoggerProvider()
provider.add_log_record_processor(log_record_processor)

logger = logging.getLogger("different_msg_types")
logger.addHandler(LoggingHandler(logger_provider=provider))

logger.warning("warning message: %s", "possible upcoming heatwave")
logger.error("Very high rise in temperatures across the globe")
logger.critical("Temperature hits high 420 C in Hyderabad")
logger.warning(["list", "of", "strings"])
logger.error({"key": "value"})
log_record_processor.shutdown()

finished_logs = exporter.get_finished_logs()
expected = [
("warning message: possible upcoming heatwave", "WARNING"),
("Very high rise in temperatures across the globe", "ERROR"),
(
"Temperature hits high 420 C in Hyderabad",
"CRITICAL",
),
(["list", "of", "strings"], "WARNING"),
({"key": "value"}, "ERROR"),
]
emitted = [
(item.log_record.body, item.log_record.severity_text)
for item in finished_logs
]
self.assertEqual(expected, emitted)


class TestBatchLogRecordProcessor(ConcurrencyTestBase):
def test_emit_call_log_record(self):
Expand Down