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

urllib instrumentation fails for local file access #820

Closed
gregbuehler opened this issue Dec 3, 2021 · 5 comments · Fixed by #823
Closed

urllib instrumentation fails for local file access #820

gregbuehler opened this issue Dec 3, 2021 · 5 comments · Fixed by #823

Comments

@gregbuehler
Copy link
Contributor

When reading local files the status code is not specified and is None. This isn't handled by the instrumentation and causes an exception.

code_ = result.getcode()
labels[SpanAttributes.HTTP_STATUS_CODE] = str(code_)
if span.is_recording():
span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, code_)
span.set_status(Status(http_status_to_status_code(code_)))

gregbuehler added a commit to gregbuehler/opentelemetry-python-contrib that referenced this issue Dec 3, 2021
@owais
Copy link
Contributor

owais commented Dec 3, 2021

Thanks for reporting this. Could you please share the exception traceback and a reproducible code example?

@gregbuehler
Copy link
Contributor Author

gregbuehler commented Dec 3, 2021

Traceback:

Traceback (most recent call last):
  File "/opt/foo/lib/python3.7/site-packages/swagger_spec_validator/common.py", line 36, in wrapper
    return method(*args, **kwargs)
  File "/opt/foo/lib/python3.7/site-packages/swagger_spec_validator/validator20.py", line 195, in validate_json
    schema, schema_path = read_resource_file(schema_path)
  File "/opt/foo/lib/python3.7/site-packages/swagger_spec_validator/common.py", line 63, in read_resource_file
    return read_file(schema_path), schema_path
  File "/opt/foo/lib/python3.7/site-packages/swagger_spec_validator/common.py", line 57, in read_file
    return read_url(get_uri_from_file_path(file_path))
  File "/opt/foo/lib/python3.7/site-packages/swagger_spec_validator/common.py", line 67, in read_url
    with contextlib.closing(urlopen(url, timeout=timeout)) as fh:
  File "/usr/lib/python3.7/urllib/request.py", line 222, in urlopen
    return opener.open(url, data, timeout)
  File "/opt/foo/lib/python3.7/site-packages/opentelemetry/instrumentation/urllib/__init__.py", line 163, in instrumented_open
    opener, request_, call_wrapped, get_or_create_headers
  File "/opt/foo/lib/python3.7/site-packages/opentelemetry/instrumentation/urllib/__init__.py", line 217, in _instrumented_open_call
    span.set_status(Status(http_status_to_status_code(code_)))
  File "/opt/foo/lib/python3.7/site-packages/opentelemetry/instrumentation/utils.py", line 49, in http_status_to_status_code
    if status < 100:
TypeError: '<' not supported between instances of 'NoneType' and 'int'

@gregbuehler
Copy link
Contributor Author

It's not minimal, but reproduction is as easy as specifying a local file as the schema for https://github.com/Yelp/swagger_spec_validator.

@lzchen
Copy link
Contributor

lzchen commented Dec 3, 2021

Is "returning a non-int status code' a common thing amongst instrumentations? If not, maybe we should treat this as a special use case and address it only for urlib.

gregbuehler added a commit to gregbuehler/opentelemetry-python-contrib that referenced this issue Dec 3, 2021
@owais
Copy link
Contributor

owais commented Dec 4, 2021

I think in the interest of never crashing, http_status_to_status_code should be defensive and return non-error in case it is passed invalid input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants