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

Instrumentation runtime checks #475

Merged
merged 2 commits into from
May 27, 2021
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
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
uses: actions/cache@v2
with:
path: .tox
key: tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}
key: v2-build-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the rename for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.tox dir was cached and resulted in failed builds. Had to change the key in order to invalidate it.

- name: run tox
run: tox -f ${{ matrix.python-version }}-${{ matrix.package }} -- --benchmark-json=${{ env.RUN_MATRIX_COMBINATION }}-benchmark.json
- name: Find and merge benchmarks
Expand Down Expand Up @@ -99,6 +99,6 @@ jobs:
uses: actions/cache@v2
with:
path: .tox
key: v2-tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt', 'docs-requirements.txt') }}
key: v2-misc-tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt', 'docs-requirements.txt') }}
- name: run tox
run: tox -e ${{ matrix.tox-environment }}
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [0.21b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.2.0-0.21b0) - 2021-05-11

### Changed
- Instrumentation packages don't specify the libraries they instrument as dependencies
anymore. Instead, they verify the correct version of libraries are installed at runtime.
([#475](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/475))
- `opentelemetry-propagator-ot-trace` Use `TraceFlags` object in `extract`
([#472](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/472))
- Set the `traced_request_attrs` of FalconInstrumentor by an argument correctly.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def export(self) -> None:
self.flush_condition.notify()

def _drain_queue(self):
""""Export all elements until queue is empty.
"""Export all elements until queue is empty.

Can only be called from the worker thread context because it invokes
`export` that is not thread safe.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ install_requires =
opentelemetry-api == 1.3.0.dev0
opentelemetry-semantic-conventions == 0.22.dev0
opentelemetry-instrumentation == 0.22.dev0
aiohttp ~= 3.0
wrapt >= 1.0.0, < 2.0.0

[options.packages.find]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,24 @@


import os
from configparser import ConfigParser

import setuptools

config = ConfigParser()
config.read("setup.cfg")

# We provide extras_require parameter to setuptools.setup later which
# overwrites the extra_require section from setup.cfg. To support extra_require
# secion in setup.cfg, we load it here and merge it with the extra_require param.
extras_require = {}
if "options.extras_require" in config:
for key, value in config["options.extras_require"].items():
extras_require[key] = [v for v in value.split("\n") if v.strip()]

BASE_DIR = os.path.dirname(__file__)
PACKAGE_INFO = {}

VERSION_FILENAME = os.path.join(
BASE_DIR,
"src",
Expand All @@ -30,8 +44,28 @@
"aiohttp_client",
"version.py",
)
PACKAGE_INFO = {}
with open(VERSION_FILENAME) as f:
exec(f.read(), PACKAGE_INFO)

setuptools.setup(version=PACKAGE_INFO["__version__"])
PACKAGE_FILENAME = os.path.join(
BASE_DIR,
"src",
"opentelemetry",
"instrumentation",
"aiohttp_client",
"package.py",
Copy link
Contributor

Choose a reason for hiding this comment

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

will renaming the version.py files break the release process? see eachdist.py: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/v0.20b0/scripts/eachdist.py#L578

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Will fix.

)
with open(PACKAGE_FILENAME) as f:
exec(f.read(), PACKAGE_INFO)

# Mark any instruments/runtime dependencies as test dependencies as well.
extras_require["instruments"] = PACKAGE_INFO["_instruments"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be a dumb question but why can't we put the _instruments directly in the options.extras_require inside setup.cfg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can but this data is used in two places. In setup.cfg/py for pip to use and by instrumentor to do runtime checks. It is very similar to how we refer to __version__ of each package. It is needed by setup.py and also by the python code in package itself. So we store it in python and read it from the Python code inside setup.py.

We can still store it only in setup.cfg and then use a module like pkg_resource to read the package metadata and load this information but I think that'd be unnecessary complexity and possibly perf issues.

test_deps = extras_require.get("test", [])
for dep in extras_require["instruments"]:
test_deps.append(dep)

extras_require["test"] = test_deps

setuptools.setup(
version=PACKAGE_INFO["__version__"], extras_require=extras_require
)
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,14 @@ def strip_query_params(url: yarl.URL) -> str:

import types
import typing
from typing import Collection

import aiohttp
import wrapt

from opentelemetry import context as context_api
from opentelemetry import trace
from opentelemetry.instrumentation.aiohttp_client.package import _instruments
from opentelemetry.instrumentation.aiohttp_client.version import __version__
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.instrumentation.utils import (
Expand Down Expand Up @@ -288,6 +290,9 @@ class AioHttpClientInstrumentor(BaseInstrumentor):
See `BaseInstrumentor`
"""

def instrumentation_dependencies(self) -> Collection[str]:
return _instruments

def _instrument(self, **kwargs):
"""Instruments aiohttp ClientSession

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Copyright 2020, OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


_instruments = ("aiohttp ~= 3.0",)
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ install_requires =
opentelemetry-semantic-conventions == 0.22.dev0
opentelemetry-instrumentation-dbapi == 0.22.dev0
opentelemetry-instrumentation == 0.22.dev0
aiopg >= 0.13.0
wrapt >= 1.0.0, < 2.0.0

[options.extras_require]
Expand Down
33 changes: 31 additions & 2 deletions instrumentation/opentelemetry-instrumentation-aiopg/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,44 @@


import os
from configparser import ConfigParser

import setuptools

config = ConfigParser()
config.read("setup.cfg")

# We provide extras_require parameter to setuptools.setup later which
# overwrites the extra_require section from setup.cfg. To support extra_require
# secion in setup.cfg, we load it here and merge it with the extra_require param.
extras_require = {}
if "options.extras_require" in config:
for key, value in config["options.extras_require"].items():
extras_require[key] = [v for v in value.split("\n") if v.strip()]

BASE_DIR = os.path.dirname(__file__)
PACKAGE_INFO = {}

VERSION_FILENAME = os.path.join(
BASE_DIR, "src", "opentelemetry", "instrumentation", "aiopg", "version.py"
)
PACKAGE_INFO = {}
with open(VERSION_FILENAME) as f:
exec(f.read(), PACKAGE_INFO)

setuptools.setup(version=PACKAGE_INFO["__version__"])
PACKAGE_FILENAME = os.path.join(
BASE_DIR, "src", "opentelemetry", "instrumentation", "aiopg", "package.py"
)
with open(PACKAGE_FILENAME) as f:
exec(f.read(), PACKAGE_INFO)

# Mark any instruments/runtime dependencies as test dependencies as well.
extras_require["instruments"] = PACKAGE_INFO["_instruments"]
test_deps = extras_require.get("test", [])
for dep in extras_require["instruments"]:
test_deps.append(dep)

extras_require["test"] = test_deps

setuptools.setup(
version=PACKAGE_INFO["__version__"], extras_require=extras_require
)
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@
---
"""

from typing import Collection

from opentelemetry.instrumentation.aiopg import wrappers
from opentelemetry.instrumentation.aiopg.package import _instruments
from opentelemetry.instrumentation.aiopg.version import __version__
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor

Expand All @@ -60,6 +63,9 @@ class AiopgInstrumentor(BaseInstrumentor):

_DATABASE_SYSTEM = "postgresql"

def instrumentation_dependencies(self) -> Collection[str]:
return _instruments

def _instrument(self, **kwargs):
"""Integrate with PostgreSQL aiopg library.
aiopg: https://github.com/aio-libs/aiopg
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Copyright The OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


_instruments = ("aiopg >= 0.13.0",)
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ install_requires =
opentelemetry-api == 1.3.0.dev0
opentelemetry-semantic-conventions == 0.22.dev0
opentelemetry-instrumentation == 0.22.dev0
asgiref ~= 3.0

[options.extras_require]
test =
Expand Down
33 changes: 31 additions & 2 deletions instrumentation/opentelemetry-instrumentation-asgi/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,44 @@


import os
from configparser import ConfigParser

import setuptools

config = ConfigParser()
config.read("setup.cfg")

# We provide extras_require parameter to setuptools.setup later which
# overwrites the extra_require section from setup.cfg. To support extra_require
# secion in setup.cfg, we load it here and merge it with the extra_require param.
extras_require = {}
if "options.extras_require" in config:
for key, value in config["options.extras_require"].items():
extras_require[key] = [v for v in value.split("\n") if v.strip()]

BASE_DIR = os.path.dirname(__file__)
PACKAGE_INFO = {}

VERSION_FILENAME = os.path.join(
BASE_DIR, "src", "opentelemetry", "instrumentation", "asgi", "version.py"
)
PACKAGE_INFO = {}
with open(VERSION_FILENAME) as f:
exec(f.read(), PACKAGE_INFO)

setuptools.setup(version=PACKAGE_INFO["__version__"])
PACKAGE_FILENAME = os.path.join(
BASE_DIR, "src", "opentelemetry", "instrumentation", "asgi", "package.py"
)
with open(PACKAGE_FILENAME) as f:
exec(f.read(), PACKAGE_INFO)

# Mark any instruments/runtime dependencies as test dependencies as well.
extras_require["instruments"] = PACKAGE_INFO["_instruments"]
test_deps = extras_require.get("test", [])
for dep in extras_require["instruments"]:
test_deps.append(dep)

extras_require["test"] = test_deps

setuptools.setup(
version=PACKAGE_INFO["__version__"], extras_require=extras_require
)
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ def collect_request_attributes(scope):


def get_host_port_url_tuple(scope):
"""Returns (host, port, full_url) tuple.
"""
"""Returns (host, port, full_url) tuple."""
server = scope.get("server") or ["0.0.0.0", 80]
port = server[1]
server_host = server[0] + (":" + str(port) if port != 80 else "")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Copyright The OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


_instruments = ("asgiref ~= 3.0",)
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ install_requires =
opentelemetry-api == 1.3.0.dev0
opentelemetry-semantic-conventions == 0.22.dev0
opentelemetry-instrumentation == 0.22.dev0
asyncpg >= 0.12.0

[options.extras_require]
test =
Expand Down
38 changes: 36 additions & 2 deletions instrumentation/opentelemetry-instrumentation-asyncpg/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,24 @@


import os
from configparser import ConfigParser

import setuptools

config = ConfigParser()
config.read("setup.cfg")

# We provide extras_require parameter to setuptools.setup later which
# overwrites the extra_require section from setup.cfg. To support extra_require
# secion in setup.cfg, we load it here and merge it with the extra_require param.
extras_require = {}
if "options.extras_require" in config:
for key, value in config["options.extras_require"].items():
extras_require[key] = [v for v in value.split("\n") if v.strip()]

BASE_DIR = os.path.dirname(__file__)
PACKAGE_INFO = {}

VERSION_FILENAME = os.path.join(
BASE_DIR,
"src",
Expand All @@ -30,8 +44,28 @@
"asyncpg",
"version.py",
)
PACKAGE_INFO = {}
with open(VERSION_FILENAME) as f:
exec(f.read(), PACKAGE_INFO)

setuptools.setup(version=PACKAGE_INFO["__version__"])
PACKAGE_FILENAME = os.path.join(
BASE_DIR,
"src",
"opentelemetry",
"instrumentation",
"asyncpg",
"package.py",
)
with open(PACKAGE_FILENAME) as f:
exec(f.read(), PACKAGE_INFO)

# Mark any instruments/runtime dependencies as test dependencies as well.
extras_require["instruments"] = PACKAGE_INFO["_instruments"]
test_deps = extras_require.get("test", [])
for dep in extras_require["instruments"]:
test_deps.append(dep)

extras_require["test"] = test_deps

setuptools.setup(
version=PACKAGE_INFO["__version__"], extras_require=extras_require
)
Loading