Implement new HTTP semantic convention opt-in for Falcon #2790

FilipNikolovski · 2024-08-09T15:22:39Z

Description

This change updates the falcon instrumentor to adhere to the new HTTP semantic conventions, according to the migration plan outlined here:

also fixes #1532

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Updated the tests to assert that the new span and duration attributes have been added.

emdneto

A big thanks for working on this one. Just some comments

instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py

...on/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py

CHANGELOG.md

…ests

xrmx · 2024-12-03T10:02:25Z

@FilipNikolovski tox -e generate step is failing, adding _semconv_status = "migration" in package.py should be enough

…ki/opentelemetry-python-contrib into falcon_new_semconv

FilipNikolovski · 2024-12-03T10:21:13Z

@FilipNikolovski tox -e generate step is failing, adding _semconv_status = "migration" in package.py should be enough

Thanks for the feedback. I've updated the file, but that step is still failing 🤷. Not sure if I need to run tox -e generate locally or not?

xrmx · 2024-12-03T11:00:52Z

@FilipNikolovski tox -e generate step is failing, adding _semconv_status = "migration" in package.py should be enough

Thanks for the feedback. I've updated the file, but that step is still failing 🤷. Not sure if I need to run tox -e generate locally or not?

You need to run it locally and commit the updated files so it will match on CI

...on/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py

lzchen · 2024-12-03T18:01:56Z

...on/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py

-                duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = (
-                    span.attributes.get(SpanAttributes.HTTP_STATUS_CODE)
+            duration_s = default_timer() - start
+            if self.duration_histogram_old:


Nit: I think you can use add_response_attributes instead to populate duration attributes with status.

Not sure if we'll be able to use this one, since it returns early if span is not recording.

Should be fixed with: https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3148/files

I've made the necessary updates.

…ndependent from tracing decisions; update tests

FilipNikolovski · 2024-12-10T11:48:49Z

@xrmx running tox -e generate throws an error for me:

Could not get hatch version from path /opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-sklearn
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
│ ages/hatch/cli/__init__.py:221 in main                                                           │
│                                                                                                  │
│   218                                                                                            │
│   219 def main():  # no cov                                                                      │
│   220 │   try:                                                                                   │
    main()
  File "/opentelemetry-python-contrib/scripts/generate_instrumentation_bootstrap.py", line 70, in main
    for pkg in get_instrumentation_packages():
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opentelemetry-python-contrib/scripts/otel_packaging.py", line 51, in get_instrumentation_packages
    raise exc
  File "/opentelemetry-python-contrib/scripts/otel_packaging.py", line 42, in get_instrumentation_packages
    version = subprocess.check_output(
              ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.pyenv/versions/3.12.7/lib/python3.12/subprocess.py", line 466, in check_output
│ ❱ 221 │   │   hatch(prog_name='hatch', windows_expand_args=False)                                │
│   222 │   except Exception:  # noqa: BLE001                                                      │
│   223 │   │   import sys                                                                         │
│   224                                                                                            │
│                                                                                                  │
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
│ ages/click/core.py:1157 in __call__                                                              │
│                                                                                                  │
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.pyenv/versions/3.12.7/lib/python3.12/subprocess.py", line 571, in run
│ ages/click/core.py:1078 in main                                                                  │
│                                                                                                  │
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
│ ages/click/core.py:1688 in invoke                                                                │
│                                                                                                  │
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
│ ages/click/core.py:1434 in invoke                                                                │
│                                                                                                  │
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
│ ages/click/core.py:783 in invoke                                                                 │
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command 'hatch version' returned non-zero exit status 1.
│                                                                                                  │
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
│ ages/click/decorators.py:45 in new_func                                                          │
│                                                                                                  │
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
│ ages/hatch/cli/version/__init__.py:35 in version                                                 │
│                                                                                                  │
│   32 │   │   if not (                                                                            │
│   33 │   │   │   'version' in app.project.metadata.dynamic or app.project.metadata.hatch.meta    │
│   34 │   │   ) or dependencies_in_sync(app.project.metadata.build.requires_complex):             │
│ ❱ 35 │   │   │   source = app.project.metadata.hatch.version.source                              │
│   36 │   │   │                                                                                   │
│   37 │   │   │   version_data = source.get_version_data()                                        │
│   38 │   │   │   original_version = version_data['version']                                      │
│                                                                                                  │
│ /opentelemetry-python-contrib/.tox/generate/lib/python3.12/site-pack │
│ ages/hatchling/metadata/core.py:1425 in version                                                  │
│                                                                                                  │
│   1422 │   │   if self._version is None:                                                         │
│   1423 │   │   │   if 'version' not in self.config:                                              │
│   1424 │   │   │   │   message = 'Missing `tool.hatch.version` configuration'                    │
│ ❱ 1425 │   │   │   │   raise ValueError(message)                                                 │
│   1426 │   │   │                                                                                 │
│   1427 │   │   │   options = self.config['version']                                              │
│   1428 │   │   │   if not isinstance(options, dict):                                             │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
ValueError: Missing `tool.hatch.version` configuration

…ki/opentelemetry-python-contrib into falcon_new_semconv

emdneto · 2024-12-10T12:55:27Z

@xrmx running tox -e generate throws an error for me:

That's weird, because /opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-sklearn is deprecated for a while

FilipNikolovski · 2024-12-10T13:03:45Z

@xrmx running tox -e generate throws an error for me:

That's weird, because /opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-sklearn is deprecated for a while

Turns out I still had the sklearn directory 🤦, the command ran successfully after I deleted it.

…ki/opentelemetry-python-contrib into falcon_new_semconv

...on/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py

xrmx · 2024-12-31T15:44:01Z

CHANGELOG.md

@@ -51,6 +51,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  ([#2942](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2942))
 - `opentelemetry-instrumentation-click`: new instrumentation to trace click commands
  ([#2994](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2994))
+- `opentelemetry-instrumentation-falcon` Implement new HTTP semantic convention opt-in for Falcon


You have to move this to the unreleased section above

Ah thanks, i've made the update.

…ry-python-contrib into falcon_new_semconv

…ki/opentelemetry-python-contrib into falcon_new_semconv

emdneto · 2025-01-02T16:04:27Z

instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py

        self.client().simulate_get("/hello/756")
-        duration = max(round((default_timer() - start) * 1000), 0)


Maybe I missed something in the discussion, but why are you removing the duration itself check?

Good catch, that probably got lost in the refactor of the tests, I can re-add that check.

I've re-added the checks.

emdneto · 2025-01-02T16:12:08Z

instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py

+            SpanAttributes.HTTP_ROUTE: "/hello",
+        }
+        expected_attributes_new = {
+            SpanAttributes.HTTP_REQUEST_METHOD: method,


In other instrumentations we are using the symbols from opentelemetry.semconv.attributes.http_attributes

I'll make the change.

Implement new HTTP semantic convention opt-in for Falcon

2d68dc5

FilipNikolovski requested a review from a team August 9, 2024 15:22

fix tests for falcon 1 & 2

228897d

lzchen mentioned this pull request Aug 9, 2024

Implement migration plan for selected instrumentations #2453

Open

emdneto reviewed Aug 9, 2024

View reviewed changes

instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py Outdated Show resolved Hide resolved

...on/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py Outdated Show resolved Hide resolved

lzchen reviewed Aug 12, 2024

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

emdneto mentioned this pull request Aug 14, 2024

2677 jinja2 semconv #2798

Closed

7 tasks

FilipNikolovski added 4 commits December 2, 2024 11:12

resolve conflicts

2d70a72

add semconv status as migration for falcon in instrumentations readme

a9a30a8

set schema_url based on the opt-in mode

b3db908

add histograms based on opt-in mode; set metrics attributes; update t…

aa54865

…ests

FilipNikolovski requested a review from a team as a code owner December 2, 2024 14:37

FilipNikolovski added 3 commits December 2, 2024 15:40

run ruff

2ed1461

disable too-many-public-methods lint rule for TestFalconInstrumentation

e8d3e6c

Merge branch 'main' into falcon_new_semconv

d1e81c0

FilipNikolovski requested review from emdneto and lzchen December 3, 2024 08:49

FilipNikolovski added 2 commits December 3, 2024 11:14

add _semconv_status in package.py

1815495

Merge branch 'falcon_new_semconv' of https://github.com/FilipNikolovs…

0307008

…ki/opentelemetry-python-contrib into falcon_new_semconv

emdneto reviewed Dec 3, 2024

View reviewed changes

...on/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py Outdated Show resolved Hide resolved

Merge branch 'main' into falcon_new_semconv

a4419a6

lzchen reviewed Dec 3, 2024

View reviewed changes

...on/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py Outdated Show resolved Hide resolved

lzchen reviewed Dec 3, 2024

View reviewed changes

FilipNikolovski added 3 commits December 10, 2024 10:04

cleanup imports

7754f0d

set request attributes in env, set status code in metric attributes i…

b941569

…ndependent from tracing decisions; update tests

Merge branch 'main' into falcon_new_semconv

e14eaf9

FilipNikolovski added 3 commits December 10, 2024 13:28

fix exceptions to not be masked as 500 on every thrown exception

c44244c

Merge branch 'falcon_new_semconv' of https://github.com/FilipNikolovs…

1cecee6

…ki/opentelemetry-python-contrib into falcon_new_semconv

resolve lint issues

59586fd

run tox -e generate; fix typo

6ac9e98

FilipNikolovski requested review from lzchen and emdneto December 10, 2024 13:04

Merge branch 'main' into falcon_new_semconv

d0757ec

emdneto approved these changes Dec 16, 2024

View reviewed changes

FilipNikolovski added 3 commits December 31, 2024 12:14

sort imports

2d6b160

Merge branch 'falcon_new_semconv' of https://github.com/FilipNikolovs…

ecd351f

…ki/opentelemetry-python-contrib into falcon_new_semconv

fix tests

3d5ec55

xrmx reviewed Dec 31, 2024

View reviewed changes

...on/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py Outdated Show resolved Hide resolved

xrmx reviewed Dec 31, 2024

View reviewed changes

FilipNikolovski and others added 5 commits December 31, 2024 16:49

Merge branch 'open-telemetry:main' into falcon_new_semconv

e51f79d

Merge branch 'main' of https://github.com/FilipNikolovski/opentelemet…

bfb8a53

…ry-python-contrib into falcon_new_semconv

update changelog

606958a

Merge branch 'falcon_new_semconv' of https://github.com/FilipNikolovs…

6ee7d35

…ki/opentelemetry-python-contrib into falcon_new_semconv

Merge branch 'main' into falcon_new_semconv

e12ace6

FilipNikolovski requested a review from xrmx January 2, 2025 08:35

emdneto self-requested a review January 2, 2025 16:00

emdneto reviewed Jan 2, 2025

View reviewed changes

update tests

8b281b2

FilipNikolovski requested a review from emdneto January 2, 2025 18:34

FilipNikolovski added 4 commits January 2, 2025 19:42

disable lint check on too-many-nested-blocks in metrics test

c662eae

use add_response_attributes function from wsgi

7702a59

resolve conflicts

174e144

cleanup unused functions

241a849

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement new HTTP semantic convention opt-in for Falcon #2790

Implement new HTTP semantic convention opt-in for Falcon #2790

FilipNikolovski commented Aug 9, 2024 •

edited

Loading

emdneto left a comment

xrmx commented Dec 3, 2024

FilipNikolovski commented Dec 3, 2024

xrmx commented Dec 3, 2024 •

edited

Loading

lzchen Dec 3, 2024

FilipNikolovski Dec 10, 2024

lzchen Jan 2, 2025

FilipNikolovski Jan 3, 2025

FilipNikolovski commented Dec 10, 2024

emdneto commented Dec 10, 2024

FilipNikolovski commented Dec 10, 2024

xrmx Dec 31, 2024 •

edited

Loading

FilipNikolovski Dec 31, 2024

emdneto Jan 2, 2025

FilipNikolovski Jan 2, 2025 •

edited

Loading

FilipNikolovski Jan 3, 2025

emdneto Jan 2, 2025

FilipNikolovski Jan 2, 2025

		self.client().simulate_get("/hello/756")
		duration = max(round((default_timer() - start) * 1000), 0)

Implement new HTTP semantic convention opt-in for Falcon #2790

Are you sure you want to change the base?

Implement new HTTP semantic convention opt-in for Falcon #2790

Conversation

FilipNikolovski commented Aug 9, 2024 • edited Loading

Description

Type of change

How Has This Been Tested?

emdneto left a comment

Choose a reason for hiding this comment

xrmx commented Dec 3, 2024

FilipNikolovski commented Dec 3, 2024

xrmx commented Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FilipNikolovski commented Dec 10, 2024

emdneto commented Dec 10, 2024

FilipNikolovski commented Dec 10, 2024

xrmx Dec 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FilipNikolovski Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FilipNikolovski commented Aug 9, 2024 •

edited

Loading

xrmx commented Dec 3, 2024 •

edited

Loading

xrmx Dec 31, 2024 •

edited

Loading

FilipNikolovski Jan 2, 2025 •

edited

Loading