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

Confusing and delayed error "TypeError: bad argument type for built-in operation" when span name is not a string #3918

Open
alexmojaki opened this issue May 14, 2024 · 6 comments · May be fixed by #3955
Labels
bug Something isn't working

Comments

@alexmojaki
Copy link

Steps to reproduce

Create a span with a non-string name, then try to encode it, e.g. with the HTTP OTLPSpanExporter. For example:

from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import BatchSpanProcessor

tracer_provider = TracerProvider()
exporter = OTLPSpanExporter()
processor = BatchSpanProcessor(exporter)
tracer_provider.add_span_processor(processor)
tracer = tracer_provider.get_tracer(__name__)

span = tracer.start_span(123)
span.end()

What is the actual behavior?

It logs:

Exception while exporting Span batch.
Traceback (most recent call last):
  File "opentelemetry/sdk/trace/export/__init__.py", line 367, in _export_batch
    self.span_exporter.export(self.spans_list[:idx])  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py", line 131, in export
    serialized_data = encode_spans(spans).SerializeToString()
                      ^^^^^^^^^^^^^^^^^^^
  File "opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py", line 58, in encode_spans
    resource_spans=_encode_resource_spans(sdk_spans)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py", line 81, in _encode_resource_spans
    pb2_span = _encode_span(sdk_span)
               ^^^^^^^^^^^^^^^^^^^^^^
  File "opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py", line 109, in _encode_span
    return PB2SPan(
           ^^^^^^^^
TypeError: bad argument type for built-in operation

What is the expected behavior?

  1. Some kind of warning/exception message that's more helpful than "bad argument type for built-in operation", e.g. "span name must be a string"
  2. An indication of the source of the problem, e.g. a traceback pointing to the tracer.start_span(123) line.
  3. The span is still exported (obviously not with the original non-string span name) or at least the other spans in the batch are still exported.

Overall I suggest that the concrete opentelemetry.sdk.trace.Tracer.start_span method and/or ReadableSpan.__init__ should emit a warning if the span name is not a string and then convert the object to a string, catching any exceptions that raises. Setting an appropriate warning stacklevel is easier in Tracer.start_span, but ReadableSpan.__init__ should cover more possible code paths.

Context

While setting a non-string span name seems unusual and difficult to do accidentally, it happened in pydantic/logfire#176 when a user called logger.exception(exc) (which is normal and OK) where logger is a standard library logging.Logger hooked up to an OTEL tracer.

@alexmojaki alexmojaki added the bug Something isn't working label May 14, 2024
@soumyadeepm04
Copy link
Contributor

Hello, if this issue is not yet assigned to someone could I have a go at it?

@pmcollins
Copy link
Member

Please feel free. The contributing guide is here. If you are making progress towards submitting a PR, please report back and we can assign it to you as well.

@soumyadeepm04
Copy link
Contributor

soumyadeepm04 commented Jun 4, 2024

I tried reproducing the error but I got this:
TypeError: Cannot set opentelemetry.proto.trace.v1.Span.name to 123: 123 has type <class 'int'>, but expected one of: (<class 'bytes'>, <class 'str'>) for field Span.name
with a stack trace and I think that covers the first 2 points of the expected behavior in the issue. Please correct me if I am wrong.

Could you please guide me as to what I should work on if this is the case?

@alexmojaki
Copy link
Author

Can you show the stack trace? It should come from a different thread so I can't see how it could point to the span = tracer.start_span(123) line.

Does this only happen when using unreleased code?

@soumyadeepm04
Copy link
Contributor

soumyadeepm04 commented Jun 4, 2024

It does not point to the exact line, just says that the issue is with the span name. Sorry wasn't more clear. Here is the stack trace:

Exception while exporting Span batch.
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/google/protobuf/internal/python_message.py", line 702, in field_setter
    new_value = type_checker.CheckValue(new_value)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/google/protobuf/internal/type_checkers.py", line 211, in CheckValue
    raise TypeError(message)
TypeError: 123 has type <class 'int'>, but expected one of: (<class 'bytes'>, <class 'str'>)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/google/protobuf/internal/python_message.py", line 558, in init
    setattr(self, field_name, field_value)
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/google/protobuf/internal/python_message.py", line 704, in field_setter
    raise TypeError(
TypeError: Cannot set opentelemetry.proto.trace.v1.Span.name to 123: 123 has type <class 'int'>, but expected one of: (<class 'bytes'>, <class 'str'>)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/soumyadeepmahapatra/Documents/Test/opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py", line 367, in _export_batch
    self.span_exporter.export(self.spans_list[:idx])  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/soumyadeepmahapatra/Documents/Test/opentelemetry-python/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py", line 167, in export
    serialized_data = self._serialize_spans(spans)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/soumyadeepmahapatra/Documents/Test/opentelemetry-python/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py", line 130, in _serialize_spans
    return encode_spans(spans).SerializePartialToString()
           ^^^^^^^^^^^^^^^^^^^
  File "/Users/soumyadeepmahapatra/Documents/Test/opentelemetry-python/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py", line 56, in encode_spans
    resource_spans=_encode_resource_spans(sdk_spans)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/soumyadeepmahapatra/Documents/Test/opentelemetry-python/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py", line 79, in _encode_resource_spans
    pb2_span = _encode_span(sdk_span)
               ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/soumyadeepmahapatra/Documents/Test/opentelemetry-python/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/trace_encoder/__init__.py", line 114, in _encode_span
    return PB2SPan(
           ^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/google/protobuf/internal/python_message.py", line 560, in init
    _ReraiseTypeErrorWithFieldName(message_descriptor.name, field_name)
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/google/protobuf/internal/python_message.py", line 477, in _ReraiseTypeErrorWithFieldName
    raise exc.with_traceback(sys.exc_info()[2])
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/google/protobuf/internal/python_message.py", line 558, in init
    setattr(self, field_name, field_value)
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/google/protobuf/internal/python_message.py", line 704, in field_setter
    raise TypeError(
TypeError: Cannot set opentelemetry.proto.trace.v1.Span.name to 123: 123 has type <class 'int'>, but expected one of: (<class 'bytes'>, <class 'str'>) for field Span.name

@alexmojaki
Copy link
Author

That doesn't really cover point 2. It's trying to serialize a batch of spans, there's no way to track down which code the faulty span came from.

The reason you're getting an error message seems to come down to this code in protobuf:

# api_implementation.py

_implementation_type = None
try:
  # pylint: disable=g-import-not-at-top
  from google.protobuf.internal import _api_implementation
  # The compile-time constants in the _api_implementation module can be used to
  # switch to a certain implementation of the Python API at build time.
  _implementation_type = _ApiVersionToImplementationType(
      _api_implementation.api_version)
except ImportError:
  pass  # Unspecified by compiler flags.


def _CanImport(mod_name):
  try:
    mod = importlib.import_module(mod_name)
    # Work around a known issue in the classic bootstrap .par import hook.
    if not mod:
      raise ImportError(mod_name + ' import succeeded but was None')
    return True
  except ImportError:
    return False


if _implementation_type is None:
  if _CanImport('google._upb._message'):
    _implementation_type = 'upb'
  elif _CanImport('google.protobuf.pyext._message'):
    _implementation_type = 'cpp'
  else:
    _implementation_type = 'python'

...

def Type():
  return _implementation_type
# message_factory.py

if api_implementation.Type() == 'python':
  from google.protobuf.internal import python_message as message_impl
else:
  from google.protobuf.pyext import cpp_message as message_impl  # pylint: disable=g-import-not-at-top

hence the lines pointing to google/protobuf/internal/python_message.py.

For me, api_implementation.Type() is upb because import google._upb._message works, which I'm guessing it doesn't for you. So it depends on how protobuf was built/installed in your environment.

@soumyadeepm04 soumyadeepm04 linked a pull request Jun 5, 2024 that will close this issue
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants