Skip to content

Commit

Permalink
Remove http.status_text from span attributes (#406)
Browse files Browse the repository at this point in the history
  • Loading branch information
srikanthccv committed Apr 8, 2021
1 parent 1ee8924 commit 370952f
Show file tree
Hide file tree
Showing 17 changed files with 6 additions and 51 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `opentelemetry-instrumenation-django` now supports request and response hooks.
([#407](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/407))

### 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

- Implement context methods for `_InterceptorChannel`
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 @@ -121,7 +121,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 @@ -143,7 +142,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 @@ -178,7 +176,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 @@ -235,7 +233,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 @@ -304,7 +301,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_)))

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 @@ -269,7 +267,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 @@ -299,7 +296,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)))


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)
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()

0 comments on commit 370952f

Please sign in to comment.