Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def agent_writer(self):
self._agent_writer = AgentWriter(uds_path=url_parsed.path)
else:
raise ValueError(
"Unknown scheme `%s` for agent URL" % url_parsed.scheme
f"Unknown scheme `{url_parsed.scheme}` for agent URL"
)
return self._agent_writer

Expand Down Expand Up @@ -225,7 +225,7 @@ def _get_span_name(span):
)
span_kind_name = span.kind.name if span.kind else None
name = (
"{}.{}".format(instrumentation_name, span_kind_name)
f"{instrumentation_name}.{span_kind_name}"
if instrumentation_name and span_kind_name
else span.name
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ async def on_request_start(
return

http_method = params.method.upper()
request_span_name = "HTTP {}".format(http_method)
request_span_name = f"HTTP {http_method}"

trace_config_ctx.span = trace_config_ctx.tracer.start_span(
request_span_name, kind=SpanKind.CLIENT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def _http_request(
method: str = "GET",
status_code: int = HTTPStatus.OK,
request_handler: typing.Callable = None,
**kwargs
**kwargs,
) -> typing.Tuple[str, int]:
"""Helper to start an aiohttp test server and send an actual HTTP request to it."""

Expand Down Expand Up @@ -132,9 +132,7 @@ def test_status_codes(self):
(span_status, None),
{
SpanAttributes.HTTP_METHOD: "GET",
SpanAttributes.HTTP_URL: "http://{}:{}/test-path?query=param#foobar".format(
host, port
),
SpanAttributes.HTTP_URL: f"http://{host}:{port}/test-path?query=param#foobar",
SpanAttributes.HTTP_STATUS_CODE: int(
status_code
),
Expand Down Expand Up @@ -167,7 +165,7 @@ def test_hooks(self):
expected = "PATCH - /some/path"

def request_hook(span: Span, params: aiohttp.TraceRequestStartParams):
span.update_name("{} - {}".format(params.method, params.url.path))
span.update_name(f"{params.method} - {params.url.path}")

def response_hook(
span: Span,
Expand Down Expand Up @@ -198,7 +196,7 @@ def response_hook(
)
self.assertEqual(
span.attributes[SpanAttributes.HTTP_URL],
"http://{}:{}{}".format(host, port, path),
f"http://{host}:{port}{path}",
)
self.assertEqual(
span.attributes[SpanAttributes.HTTP_STATUS_CODE], HTTPStatus.OK
Expand Down Expand Up @@ -227,9 +225,7 @@ def strip_query_params(url: yarl.URL) -> str:
(StatusCode.UNSET, None),
{
SpanAttributes.HTTP_METHOD: "GET",
SpanAttributes.HTTP_URL: "http://{}:{}/some/path".format(
host, port
),
SpanAttributes.HTTP_URL: f"http://{host}:{port}/some/path",
SpanAttributes.HTTP_STATUS_CODE: int(HTTPStatus.OK),
},
)
Expand Down Expand Up @@ -290,9 +286,7 @@ async def request_handler(request):
(StatusCode.ERROR, None),
{
SpanAttributes.HTTP_METHOD: "GET",
SpanAttributes.HTTP_URL: "http://{}:{}/test_timeout".format(
host, port
),
SpanAttributes.HTTP_URL: f"http://{host}:{port}/test_timeout",
},
)
]
Expand All @@ -319,9 +313,7 @@ async def request_handler(request):
(StatusCode.ERROR, None),
{
SpanAttributes.HTTP_METHOD: "GET",
SpanAttributes.HTTP_URL: "http://{}:{}/test_too_many_redirects".format(
host, port
),
SpanAttributes.HTTP_URL: f"http://{host}:{port}/test_too_many_redirects",
},
)
]
Expand Down Expand Up @@ -399,7 +391,7 @@ def test_instrument(self):
span = self.assert_spans(1)
self.assertEqual("GET", span.attributes[SpanAttributes.HTTP_METHOD])
self.assertEqual(
"http://{}:{}/test-path".format(host, port),
f"http://{host}:{port}/test-path",
span.attributes[SpanAttributes.HTTP_URL],
)
self.assertEqual(200, span.attributes[SpanAttributes.HTTP_STATUS_CODE])
Expand Down Expand Up @@ -502,13 +494,13 @@ def strip_query_params(url: yarl.URL) -> str:
)
span = self.assert_spans(1)
self.assertEqual(
"http://{}:{}/test-path".format(host, port),
f"http://{host}:{port}/test-path",
span.attributes[SpanAttributes.HTTP_URL],
)

def test_hooks(self):
def request_hook(span: Span, params: aiohttp.TraceRequestStartParams):
span.update_name("{} - {}".format(params.method, params.url.path))
span.update_name(f"{params.method} - {params.url.path}")

def response_hook(
span: Span,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,9 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]:
Returns:
a tuple of the span name, and any attributes to attach to the span.
"""
span_name = scope.get("path", "").strip() or "HTTP {}".format(
scope.get("method", "").strip()
span_name = (
scope.get("path", "").strip()
or f"HTTP {scope.get('method', '').strip()}"
)

return span_name, {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def _common_request( # pylint: disable=too-many-locals
endpoint_name = getattr(instance, "host").split(".")[0]

with self._tracer.start_as_current_span(
"{}.command".format(endpoint_name), kind=SpanKind.CONSUMER,
f"{endpoint_name}.command", kind=SpanKind.CONSUMER,
) as span:
span.set_attribute("endpoint", endpoint_name)
if args:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
def assert_span_http_status_code(span, code):
"""Assert on the span's 'http.status_code' tag"""
tag = span.attributes[SpanAttributes.HTTP_STATUS_CODE]
assert tag == code, "%r != %r" % (tag, code)
assert tag == code, f"{tag} != {code}"


class TestBotoInstrumentor(TestBase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def _patched_api_call(self, original_func, instance, args, kwargs):
result = None

with self._tracer.start_as_current_span(
"{}".format(service_name), kind=SpanKind.CLIENT,
f"{service_name}", kind=SpanKind.CLIENT,
) as span:
# inject trace context into payload headers for lambda Invoke
if BotocoreInstrumentor._is_lambda_invoke(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def _trace_prerun(self, *args, **kwargs):

logger.debug("prerun signal start task_id=%s", task_id)

operation_name = "{0}/{1}".format(_TASK_RUN, task.name)
operation_name = f"{_TASK_RUN}/{task.name}"
span = self._tracer.start_span(
operation_name, context=tracectx, kind=trace.SpanKind.CONSUMER
)
Expand Down Expand Up @@ -178,7 +178,7 @@ def _trace_before_publish(self, *args, **kwargs):
if task is None or task_id is None:
return

operation_name = "{0}/{1}".format(_TASK_APPLY_ASYNC, task.name)
operation_name = f"{_TASK_APPLY_ASYNC}/{task.name}"
span = self._tracer.start_span(
operation_name, kind=trace.SpanKind.PRODUCER
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def set_attributes_from_context(span, context):

# set attribute name if not set specially for a key
if attribute_name is None:
attribute_name = "celery.{}".format(key)
attribute_name = f"celery.{key}"

span.set_attribute(attribute_name, value)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def fn_task():
utils.retrieve_span(fn_task, task_id), (None, None)
)
except Exception as ex: # pylint: disable=broad-except
self.fail("Exception was raised: %s" % ex)
self.fail(f"Exception was raised: {ex}")

def test_task_id_from_protocol_v1(self):
# ensures a `task_id` is properly returned when Protocol v1 is used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def _get_span_name(request):
return match.view_name

except Resolver404:
return "HTTP {}".format(request.method)
return f"HTTP {request.method}"

def process_request(self, request):
# request.META is a dictionary containing all available HTTP headers
Expand Down Expand Up @@ -213,7 +213,7 @@ def process_response(self, request, response):
if activation and span:
add_response_attributes(
span,
"{} {}".format(response.status_code, response.reason_phrase),
f"{response.status_code} {response.reason_phrase}",
response,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,7 @@ def test_trace_response_headers(self):
)
self.assertEqual(
response.headers["traceresponse"],
"00-{0}-{1}-01".format(
Copy link
Member

@aabmass aabmass Sep 23, 2021

Choose a reason for hiding this comment

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

Cases like these where you don't have short variable, f-strings can be pretty ugly :( imo it's pretty unreadable compared to the format equivalent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hear that... I don't feel strongly either ways. I found the formatting previously there confusing as well w/ the multiple 0's and 1's 😄

Could create a temporary variable or just disable pylint for this line.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably more readable when it's not complex code buried in a string - I prefer the format one.

Copy link
Member

Choose a reason for hiding this comment

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

I found the formatting previously there confusing as well w/ the multiple 0's and 1's

I agree, maybe short variable names with f-strings would be better here, or could do "00-{}-{}-01".format(...) or named format params. I'll leave it up to you, don't want to block this @codeboten

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm happy to fix this in a following PR. I will refactor all the tests that are duplicated into a single test in the base class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will use the following method to simplify the code open-telemetry/opentelemetry-python#2159

format_trace_id(span.get_span_context().trace_id),
format_span_id(span.get_span_context().span_id),
),
f"00-{format_trace_id(span.get_span_context().trace_id)}-{format_span_id(span.get_span_context().span_id)}-01",
)
self.memory_exporter.clear()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ def wrapper(wrapped, _, args, kwargs):
for member in _ATTRIBUTES_FROM_RESULT:
if member in rv:
span.set_attribute(
"elasticsearch.{0}".format(member),
str(rv[member]),
f"elasticsearch.{member}", str(rv[member]),
)

if callable(response_hook):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def _test_trace_error(self, code, exc):
self.assertFalse(span.status.is_ok)
self.assertEqual(span.status.status_code, code)
self.assertEqual(
span.status.description, "{}: {}".format(type(exc).__name__, exc)
span.status.description, f"{type(exc).__name__}: {exc}"
)

def test_parent(self, request_mock):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,7 @@ def process_resource(self, req, resp, resource, params):

resource_name = resource.__class__.__name__
span.set_attribute("falcon.resource", resource_name)
span.update_name(
"{0}.on_{1}".format(resource_name, req.method.lower())
)
span.update_name(f"{resource_name}.on_{req.method.lower()}")

def process_response(
self, req, resp, resource, req_succeeded=None
Expand Down Expand Up @@ -294,6 +292,7 @@ def process_response(
else:
status = "500"
reason = "{}: {}".format(exc_type.__name__, exc)
reason = f"{exc_type.__name__}: {exc}"

status = status.split(" ")[0]
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ def _test_method(self, method):
spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1)
span = spans[0]
self.assertEqual(
span.name, "HelloWorldResource.on_{0}".format(method.lower())
)
self.assertEqual(span.name, f"HelloWorldResource.on_{method.lower()}")
self.assertEqual(span.status.status_code, StatusCode.UNSET)
self.assertEqual(
span.status.description, None,
Expand Down Expand Up @@ -209,10 +207,7 @@ def test_trace_response(self):
)
self.assertEqual(
headers["traceresponse"],
"00-{0}-{1}-01".format(
format_trace_id(span.get_span_context().trace_id),
format_span_id(span.get_span_context().span_id),
),
f"00-{format_trace_id(span.get_span_context().trace_id)}-{format_span_id(span.get_span_context().span_id)}-01",
)

set_global_response_propagator(orig)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,7 @@ def test_trace_response(self):
)
self.assertEqual(
headers["traceresponse"],
"00-{0}-{1}-01".format(
trace.format_trace_id(span.get_span_context().trace_id),
trace.format_span_id(span.get_span_context().span_id),
),
f"00-{trace.format_trace_id(span.get_span_context().trace_id)}-{trace.format_span_id(span.get_span_context().span_id)}-01",
)

set_global_response_propagator(orig)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def _intercept(self, request, metadata, client_info, invoker):
span.set_status(
Status(
status_code=StatusCode.ERROR,
description="{}: {}".format(type(exc).__name__, exc),
description=f"{type(exc).__name__}: {exc}",
)
)
span.record_exception(exc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ def abort(self, code, details):
)
self._active_span.set_status(
Status(
status_code=StatusCode.ERROR,
description="{}:{}".format(code, details),
status_code=StatusCode.ERROR, description=f"{code}:{details}",
)
)
return self._servicer_context.abort(code, details)
Expand All @@ -142,7 +141,7 @@ def set_code(self, code):
self._active_span.set_status(
Status(
status_code=StatusCode.ERROR,
description="{}:{}".format(code, details),
description=f"{code}:{details}",
)
)
return self._servicer_context.set_code(code)
Expand All @@ -153,7 +152,7 @@ def set_details(self, details):
self._active_span.set_status(
Status(
status_code=StatusCode.ERROR,
description="{}:{}".format(self.code, details),
description=f"{self.code}:{details}",
)
)
return self._servicer_context.set_details(details)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,6 @@ def create_test_server(port):
TestServer(), server
)

server.add_insecure_port("localhost:{}".format(port))
server.add_insecure_port(f"localhost:{port}")

return server
Loading