Skip to content

Don't set STATUS on SpanKind SERVER for 4XX status#776

Merged
lzchen merged 6 commits intoopen-telemetry:mainfrom
lzchen:server
Oct 26, 2021
Merged

Don't set STATUS on SpanKind SERVER for 4XX status#776
lzchen merged 6 commits intoopen-telemetry:mainfrom
lzchen:server

Conversation

@lzchen
Copy link
Copy Markdown
Contributor

@lzchen lzchen commented Oct 25, 2021

From spec

For HTTP status codes in the 4xx range span status MUST be left unset in case of SpanKind.SERVER

@lzchen lzchen requested a review from owais October 25, 2021 20:15
@lzchen lzchen self-assigned this Oct 25, 2021
@lzchen lzchen requested a review from a team October 25, 2021 20:15
@lzchen lzchen removed their assignment Oct 25, 2021
@lzchen lzchen changed the title Don't set STATUS on SpanKind SERVER Don't set STATUS on SpanKind SERVER for 4XX status Oct 25, 2021
Status(http_status_to_status_code(int(params.response.status)))
Status(
http_status_to_status_code(
int(params.response.status), server_span=True
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be excluded since this is HTTP Client?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, good catch.

@lzchen lzchen merged commit 76a5cda into open-telemetry:main Oct 26, 2021
@lzchen lzchen deleted the server branch October 26, 2021 18:44
@talboren
Copy link
Copy Markdown

talboren commented Nov 15, 2021

@lzchen @lonewolf3739 @owais
we're getting the following error message (that fails some of our requests) ever since this change was inserted (as far as I understand):

Traceback (most recent call last):
File "/home/anecdotes/.local/lib/python3.7/site-packages/uvicorn/protocols/http/httptools_impl.py", line 375, in run_asgi
result = await app(self.scope, self.receive, self.send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/uvicorn/middleware/proxy_headers.py", line 75, in call
return await self.app(scope, receive, send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/fastapi/applications.py", line 208, in call
await super().call(scope, receive, send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/applications.py", line 112, in call
await self.middleware_stack(scope, receive, send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/middleware/errors.py", line 181, in call
raise exc
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/middleware/errors.py", line 159, in call
await self.app(scope, receive, _send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/middleware/cors.py", line 84, in call
await self.app(scope, receive, send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/middleware/base.py", line 57, in call
task_group.cancel_scope.cancel()
File "/home/anecdotes/.local/lib/python3.7/site-packages/anyio/_backends/_asyncio.py", line 567, in aexit
raise exceptions[0]
File "/home/anecdotes/.local/lib/python3.7/site-packages/anyio/_backends/_asyncio.py", line 604, in _run_wrapped_task
await coro
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/middleware/base.py", line 30, in coro
await self.app(scope, request.receive, send_stream.send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/opentelemetry/instrumentation/asgi/init.py", line 346, in call
await self.app(scope, wrapped_receive, wrapped_send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/middleware/base.py", line 56, in call
await response(scope, receive, send)
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/responses.py", line 227, in call
await wrap(partial(self.listen_for_disconnect, receive))
File "/home/anecdotes/.local/lib/python3.7/site-packages/anyio/_backends/_asyncio.py", line 567, in aexit
raise exceptions[0]
File "/home/anecdotes/.local/lib/python3.7/site-packages/anyio/_backends/_asyncio.py", line 604, in _run_wrapped_task
await coro
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/responses.py", line 223, in wrap
await func()
File "/home/anecdotes/.local/lib/python3.7/site-packages/starlette/responses.py", line 209, in stream_response
"headers": self.raw_headers,
File "/home/anecdotes/.local/lib/python3.7/site-packages/opentelemetry/instrumentation/asgi/init.py", line 338, in wrapped_send
set_status_code(span, status_code)
File "/home/anecdotes/.local/lib/python3.7/site-packages/opentelemetry/instrumentation/asgi/init.py", line 222, in set_status_code
Status(http_status_to_status_code(status_code, server_span=True))
TypeError: http_status_to_status_code() got an unexpected keyword argument 'server_span'

any chance you guys have some idea? not sure what further details to provide

@owais
Copy link
Copy Markdown
Contributor

owais commented Nov 15, 2021

@talboren could you please create an issue for this and share the output of pip freeze there? It looks like you might be using an older version of opentelemetry-instrumentation package.

@talboren
Copy link
Copy Markdown

@talboren could you please create an issue for this and share the output of pip freeze there? It looks like you might be using an older version of opentelemetry-instrumentation package.

That was exactly it.
We have a pre-built docker image that includes some of our requirements and is being built on a weekly-basis, just updated it and everything seems to be working fine, thanks for opening my eyes ;)

@talboren
Copy link
Copy Markdown

talboren commented Nov 15, 2021

@talboren could you please create an issue for this and share the output of pip freeze there? It looks like you might be using an older version of opentelemetry-instrumentation package.

That was exactly it. We have a pre-built docker image that includes some of our requirements and is being built on a weekly-basis, just updated it and everything seems to be working fine, thanks for opening my eyes ;)

@owais so seems like it wasn't that, I'm opening an issue and will post pip freeze results in there.

#811

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 this pull request may close these issues.

4 participants