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

Remove http.status_text from span attributes #406

Merged
merged 7 commits into from
Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `opentelemetry-instrumentation-urllib3` Add urllib3 instrumentation
([#299](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/299))

### Removed
- Remove `http.status_text` from span attributes
([#406](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/406))

## [0.19b0](https://github.com/open-telemetry/opentelemetry-python-contrib/releases/tag/v0.19b0) - 2021-03-26

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,6 @@ async def on_request_end(
trace_config_ctx.span.set_attribute(
"http.status_code", params.response.status
)
trace_config_ctx.span.set_attribute(
"http.status_text", params.response.reason
)
_end_trace(trace_config_ctx)

async def on_request_exception(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ def test_status_codes(self):
host, port
),
"http.status_code": int(status_code),
"http.status_text": status_code.phrase,
},
)
]
Expand Down Expand Up @@ -191,7 +190,6 @@ def test_span_name_option(self):
host, port, path
),
"http.status_code": int(HTTPStatus.OK),
"http.status_text": HTTPStatus.OK.phrase,
},
)
]
Expand Down Expand Up @@ -222,7 +220,6 @@ def strip_query_params(url: yarl.URL) -> str:
host, port
),
"http.status_code": int(HTTPStatus.OK),
"http.status_text": HTTPStatus.OK.phrase,
},
)
]
Expand Down Expand Up @@ -361,7 +358,6 @@ def test_instrument(self):
span.attributes["http.url"],
)
self.assertEqual(200, span.attributes["http.status_code"])
self.assertEqual("OK", span.attributes["http.status_text"])

def test_instrument_with_existing_trace_config(self):
trace_config = aiohttp.TraceConfig()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ def test_templated_route_get(self):
)
self.assertEqual(span.attributes["http.scheme"], "http")
self.assertEqual(span.attributes["http.status_code"], 200)
self.assertEqual(span.attributes["http.status_text"], "OK")

def test_traced_get(self):
Client().get("/traced/")
Expand All @@ -138,7 +137,6 @@ def test_traced_get(self):
self.assertEqual(span.attributes["http.route"], "^traced/")
self.assertEqual(span.attributes["http.scheme"], "http")
self.assertEqual(span.attributes["http.status_code"], 200)
self.assertEqual(span.attributes["http.status_text"], "OK")

def test_not_recording(self):
mock_tracer = Mock()
Expand Down Expand Up @@ -173,7 +171,6 @@ def test_traced_post(self):
self.assertEqual(span.attributes["http.route"], "^traced/")
self.assertEqual(span.attributes["http.scheme"], "http")
self.assertEqual(span.attributes["http.status_code"], 200)
self.assertEqual(span.attributes["http.status_text"], "OK")

def test_error(self):
with self.assertRaises(ValueError):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ def _test_method(self, method):
"net.peer.port": "65133",
"http.flavor": "1.1",
"falcon.resource": "HelloWorldResource",
"http.status_text": "Created",
"http.status_code": 201,
},
)
Expand All @@ -129,7 +128,6 @@ def test_404(self):
"net.peer.ip": "127.0.0.1",
"net.peer.port": "65133",
"http.flavor": "1.1",
"http.status_text": "Not Found",
"http.status_code": 404,
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def expected_attributes(override_attributes):
"http.host": "localhost",
"http.target": "/",
"http.flavor": "1.1",
"http.status_text": "OK",
"http.status_code": 200,
}
for key, val in override_attributes.items():
Expand Down Expand Up @@ -138,7 +137,6 @@ def test_404(self):
{
"http.method": "POST",
"http.target": "/bye",
"http.status_text": "NOT FOUND",
"http.status_code": 404,
}
)
Expand All @@ -157,7 +155,6 @@ def test_internal_error(self):
{
"http.target": "/hello/500",
"http.route": "/hello/<int:helloid>",
"http.status_text": "INTERNAL SERVER ERROR",
"http.status_code": 500,
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def expected_attributes(override_attributes):
"http.host": "localhost",
"http.target": "/",
"http.flavor": "1.1",
"http.status_text": "OK",
"http.status_code": 200,
}
for key, val in override_attributes.items():
Expand Down Expand Up @@ -118,7 +117,6 @@ def test_404(self):
{
"http.method": "POST",
"http.target": "/bye",
"http.status_text": "Not Found",
"http.status_code": 404,
}
)
Expand All @@ -137,7 +135,6 @@ def test_internal_error(self):
{
"http.target": "/hello/500",
"http.route": "/hello/{helloid}",
"http.status_text": "Internal Server Error",
"http.status_code": 500,
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ def _instrumented_requests_call(
if isinstance(result, Response):
if span.is_recording():
span.set_attribute("http.status_code", result.status_code)
span.set_attribute("http.status_text", result.reason)
span.set_status(
Status(http_status_to_status_code(result.status_code))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ def test_basic(self):
"http.method": "GET",
"http.url": self.URL,
"http.status_code": 200,
"http.status_text": "OK",
},
)

Expand Down Expand Up @@ -127,7 +126,6 @@ def test_not_foundbasic(self):
span = self.assert_span()

self.assertEqual(span.attributes.get("http.status_code"), 404)
self.assertEqual(span.attributes.get("http.status_text"), "Not Found")

self.assertIs(
span.status.status_code, trace.StatusCode.ERROR,
Expand Down Expand Up @@ -237,7 +235,6 @@ def span_callback(span, result: requests.Response):
"http.method": "GET",
"http.url": self.URL,
"http.status_code": 200,
"http.status_text": "OK",
"http.response.body": "Hello!",
},
)
Expand Down Expand Up @@ -306,7 +303,6 @@ def test_requests_exception_with_response(self, *_, **__):
"http.method": "GET",
"http.url": self.URL,
"http.status_code": 500,
"http.status_text": "Internal Server Error",
},
)
self.assertEqual(span.status.status_code, StatusCode.ERROR)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ def _finish_span(tracer, handler, error=None):
return

if ctx.span.is_recording():
if reason:
ctx.span.set_attribute("http.status_text", reason)
ctx.span.set_attribute("http.status_code", status_code)
ctx.span.set_status(
Status(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ def _test_http_method_call(self, method):
"http.host": "127.0.0.1:" + str(self.get_http_port()),
"http.target": "/",
"net.peer.ip": "127.0.0.1",
"http.status_text": "Created",
"http.status_code": 201,
},
)
Expand Down Expand Up @@ -205,7 +204,6 @@ def _test_async_handler(self, url, handler_name):
"http.host": "127.0.0.1:" + str(self.get_http_port()),
"http.target": url,
"net.peer.ip": "127.0.0.1",
"http.status_text": "Created",
"http.status_code": 201,
},
)
Expand Down Expand Up @@ -274,7 +272,6 @@ def test_404(self):
"http.host": "127.0.0.1:" + str(self.get_http_port()),
"http.target": "/missing-url",
"net.peer.ip": "127.0.0.1",
"http.status_text": "Not Found",
"http.status_code": 404,
},
)
Expand Down Expand Up @@ -318,7 +315,6 @@ def test_dynamic_handler(self):
"http.host": "127.0.0.1:" + str(self.get_http_port()),
"http.target": "/dyna",
"net.peer.ip": "127.0.0.1",
"http.status_text": "Accepted",
"http.status_code": 202,
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ def _instrumented_open_call(

if span.is_recording():
span.set_attribute("http.status_code", code_)
span.set_attribute("http.status_text", result.reason)
span.set_status(Status(http_status_to_status_code(code_)))
codeboten marked this conversation as resolved.
Show resolved Hide resolved

ver_ = str(getattr(result, "version", ""))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ def test_basic(self):
"http.method": "GET",
"http.url": self.URL,
"http.status_code": 200,
"http.status_text": "OK",
},
)

Expand Down Expand Up @@ -158,7 +157,6 @@ def test_not_foundbasic(self):
span = self.assert_span()

self.assertEqual(span.attributes.get("http.status_code"), 404)
self.assertEqual(span.attributes.get("http.status_text"), "Not Found")

self.assertIs(
span.status.status_code, trace.StatusCode.ERROR,
Expand Down Expand Up @@ -271,7 +269,6 @@ def span_callback(span, result: HTTPResponse):
"http.method": "GET",
"http.url": self.URL,
"http.status_code": 200,
"http.status_text": "OK",
"http.response.body": "Hello!",
},
)
Expand Down Expand Up @@ -301,7 +298,6 @@ def test_requests_exception_with_response(self, *_, **__):
"http.method": "GET",
"http.url": "http://httpbin.org/status/500",
"http.status_code": 500,
"http.status_text": "Internal Server Error",
},
)
self.assertEqual(span.status.status_code, StatusCode.ERROR)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ def _apply_response(span: Span, response: urllib3.response.HTTPResponse):
return

span.set_attribute("http.status_code", response.status)
span.set_attribute("http.status_text", response.reason)
span.set_status(Status(http_status_to_status_code(response.status)))
codeboten marked this conversation as resolved.
Show resolved Hide resolved


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ def assert_success_span(
"http.method": "GET",
"http.url": url,
"http.status_code": 200,
"http.status_text": "OK",
}
self.assertEqual(attributes, span.attributes)

Expand Down Expand Up @@ -127,7 +126,6 @@ def test_basic_not_found(self):

span = self.assert_span()
self.assertEqual(404, span.attributes.get("http.status_code"))
self.assertEqual("Not Found", span.attributes.get("http.status_text"))
self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code)

def test_basic_http_non_default_port(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@ def add_response_attributes(
passed to a PEP3333-conforming start_response callable."""
if not span.is_recording():
return
status_code, status_text = start_response_status.split(" ", 1)
codeboten marked this conversation as resolved.
Show resolved Hide resolved
span.set_attribute("http.status_text", status_text)
status_code, _ = start_response_status.split(" ", 1)

try:
status_code = int(status_code)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ def validate_response(
"http.host": "127.0.0.1",
"http.flavor": "1.0",
"http.url": "http://127.0.0.1/",
"http.status_text": "OK",
"http.status_code": 200,
}
if http_method is not None:
Expand Down Expand Up @@ -337,20 +336,10 @@ def test_http_user_agent_attribute(self):

def test_response_attributes(self):
otel_wsgi.add_response_attributes(self.span, "404 Not Found", {})
expected = (
mock.call("http.status_code", 404),
mock.call("http.status_text", "Not Found"),
)
expected = (mock.call("http.status_code", 404),)
self.assertEqual(self.span.set_attribute.call_count, len(expected))
self.span.set_attribute.assert_has_calls(expected, any_order=True)

def test_response_attributes_invalid_status_code(self):
otel_wsgi.add_response_attributes(self.span, "Invalid Status Code", {})
self.assertEqual(self.span.set_attribute.call_count, 1)
self.span.set_attribute.assert_called_with(
"http.status_text", "Status Code"
)


if __name__ == "__main__":
unittest.main()