Skip to content

Commit

Permalink
Merge branch 'main' into audit-and-test-opentelemetry-instrumentation…
Browse files Browse the repository at this point in the history
…-elasticsearch-no-op-tracer
  • Loading branch information
shalevr authored Feb 14, 2023
2 parents 8119215 + b513d1f commit ffed02e
Show file tree
Hide file tree
Showing 15 changed files with 125 additions and 82 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/backport.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
env:
NUMBER: ${{ github.event.inputs.number }}
# not using secrets.GITHUB_TOKEN since pull requests from that token do not run workflows
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
GITHUB_TOKEN: ${{ secrets.OPENTELEMETRYBOT_GITHUB_TOKEN }}
run: |
commit=$(gh pr view $NUMBER --json mergeCommit --jq .mergeCommit.oid)
title=$(gh pr view $NUMBER --json title --jq .title)
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/prepare-patch-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
- name: Create pull request
env:
# not using secrets.GITHUB_TOKEN since pull requests from that token do not run workflows
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
GITHUB_TOKEN: ${{ secrets.OPENTELEMETRYBOT_GITHUB_TOKEN }}
run: |
message="Prepare release ${STABLE_VERSION}/${UNSTABLE_VERSION}"
branch="opentelemetrybot/prepare-release-${STABLE_VERSION}-${UNSTABLE_VERSION}"
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/prepare-release-branch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ jobs:
- name: Create pull request against the release branch
env:
# not using secrets.GITHUB_TOKEN since pull requests from that token do not run workflows
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
GITHUB_TOKEN: ${{ secrets.OPENTELEMETRYBOT_GITHUB_TOKEN }}
run: |
message="Prepare release ${STABLE_VERSION}/${UNSTABLE_VERSION}"
branch="opentelemetrybot/prepare-release-${STABLE_VERSION}-${UNSTABLE_VERSION}"
Expand Down Expand Up @@ -181,7 +181,7 @@ jobs:
- name: Create pull request against main
env:
# not using secrets.GITHUB_TOKEN since pull requests from that token do not run workflows
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
GITHUB_TOKEN: ${{ secrets.OPENTELEMETRYBOT_GITHUB_TOKEN }}
run: |
message="Update version to ${STABLE_NEXT_VERSION}/${UNSTABLE_NEXT_VERSION}"
body="Update version to \`${STABLE_NEXT_VERSION}/${UNSTABLE_NEXT_VERSION}\`."
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ jobs:
- name: Create pull request against main
env:
# not using secrets.GITHUB_TOKEN since pull requests from that token do not run workflows
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
GITHUB_TOKEN: ${{ secrets.OPENTELEMETRYBOT_GITHUB_TOKEN }}
run: |
message="Copy change log updates from $GITHUB_REF_NAME"
body="Copy log updates from \`$GITHUB_REF_NAME\`."
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#1435](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1435))
- mongo db - fix db statement capturing
([#1512](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1512))
- Add commit method for ConfluentKafkaInstrumentor's ProxiedConsumer
([#1656](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1656))

## Version 1.15.0/0.36b0 (2022-12-10)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from pkg_resources import iter_entry_points

from opentelemetry import context
from opentelemetry import trace as trace_api
from opentelemetry.instrumentation import aiohttp_client
from opentelemetry.instrumentation.aiohttp_client import (
AioHttpClientInstrumentor,
Expand Down Expand Up @@ -434,6 +435,18 @@ async def create_session(server: aiohttp.test_utils.TestServer):
run_with_test_server(create_session, self.URL, self.default_handler)
self.assert_spans(1)

def test_no_op_tracer_provider(self):
AioHttpClientInstrumentor().uninstrument()
AioHttpClientInstrumentor().instrument(
tracer_provider=trace_api.NoOpTracerProvider()
)

run_with_test_server(
self.get_default_request(), self.URL, self.default_handler
)
spans_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans_list), 0)

def test_uninstrument(self):
AioHttpClientInstrumentor().uninstrument()
run_with_test_server(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ def __init__(self, consumer: Consumer, tracer: Tracer):
def committed(self, partitions, timeout=-1):
return self._consumer.committed(partitions, timeout)

def commit(self, *args, **kwargs):
return self._consumer.commit(*args, **kwargs)

def consume(
self, num_messages=1, *args, **kwargs
): # pylint: disable=keyword-arg-before-vararg
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,18 @@ def test_instrument_api(self) -> None:

consumer = instrumentation.uninstrument_consumer(consumer)
self.assertEqual(consumer.__class__, Consumer)

def test_consumer_commit_method_exists(self) -> None:
instrumentation = ConfluentKafkaInstrumentor()

consumer = Consumer(
{
"bootstrap.servers": "localhost:29092",
"group.id": "mygroup",
"auto.offset.reset": "earliest",
}
)

consumer = instrumentation.instrument_consumer(consumer)
self.assertEqual(consumer.__class__, ProxiedConsumer)
self.assertTrue(hasattr(consumer, "commit"))
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def instrument_connection(
Returns:
An instrumented connection.
"""
if isinstance(connection, _TracedConnectionProxy):
if isinstance(connection, wrapt.ObjectProxy):
_logger.warning("Connection already instrumented")
return connection

Expand All @@ -230,8 +230,8 @@ def uninstrument_connection(connection):
Returns:
An uninstrumented connection.
"""
if isinstance(connection, _TracedConnectionProxy):
return connection._connection
if isinstance(connection, wrapt.ObjectProxy):
return connection.__wrapped__

_logger.warning("Connection is not instrumented")
return connection
Expand Down Expand Up @@ -320,22 +320,14 @@ def get_connection_attributes(self, connection):
self.span_attributes[SpanAttributes.NET_PEER_PORT] = port


class _TracedConnectionProxy:
pass


def get_traced_connection_proxy(
connection, db_api_integration, *args, **kwargs
):
# pylint: disable=abstract-method
class TracedConnectionProxy(type(connection), _TracedConnectionProxy):
def __init__(self, connection):
self._connection = connection

def __getattr__(self, name):
return object.__getattribute__(
object.__getattribute__(self, "_connection"), name
)
class TracedConnectionProxy(wrapt.ObjectProxy):
# pylint: disable=unused-argument
def __init__(self, connection, *args, **kwargs):
wrapt.ObjectProxy.__init__(self, connection)

def __getattribute__(self, name):
if object.__getattribute__(self, name):
Expand All @@ -347,16 +339,17 @@ def __getattribute__(self, name):

def cursor(self, *args, **kwargs):
return get_traced_cursor_proxy(
self._connection.cursor(*args, **kwargs), db_api_integration
self.__wrapped__.cursor(*args, **kwargs), db_api_integration
)

# For some reason this is necessary as trying to access the close
# method of self._connection via __getattr__ leads to unexplained
# errors.
def close(self):
self._connection.close()
def __enter__(self):
self.__wrapped__.__enter__()
return self

def __exit__(self, *args, **kwargs):
self.__wrapped__.__exit__(*args, **kwargs)

return TracedConnectionProxy(connection)
return TracedConnectionProxy(connection, *args, **kwargs)


class CursorTracer:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,14 +325,14 @@ def test_callproc(self):

@mock.patch("opentelemetry.instrumentation.dbapi")
def test_wrap_connect(self, mock_dbapi):
dbapi.wrap_connect(self.tracer, MockConnectionEmpty(), "connect", "-")
dbapi.wrap_connect(self.tracer, mock_dbapi, "connect", "-")
connection = mock_dbapi.connect()
self.assertEqual(mock_dbapi.connect.call_count, 1)
self.assertIsInstance(connection._connection, mock.Mock)
self.assertIsInstance(connection.__wrapped__, mock.Mock)

@mock.patch("opentelemetry.instrumentation.dbapi")
def test_unwrap_connect(self, mock_dbapi):
dbapi.wrap_connect(self.tracer, MockConnectionEmpty(), "connect", "-")
dbapi.wrap_connect(self.tracer, mock_dbapi, "connect", "-")
connection = mock_dbapi.connect()
self.assertEqual(mock_dbapi.connect.call_count, 1)

Expand All @@ -342,21 +342,19 @@ def test_unwrap_connect(self, mock_dbapi):
self.assertIsInstance(connection, mock.Mock)

def test_instrument_connection(self):
connection = MockConnectionEmpty()
connection = mock.Mock()
# Avoid get_attributes failing because can't concatenate mock
# pylint: disable=attribute-defined-outside-init
connection.database = "-"
connection2 = dbapi.instrument_connection(self.tracer, connection, "-")
self.assertIs(connection2._connection, connection)
self.assertIs(connection2.__wrapped__, connection)

def test_uninstrument_connection(self):
connection = MockConnectionEmpty()
connection = mock.Mock()
# Set connection.database to avoid a failure because mock can't
# be concatenated
# pylint: disable=attribute-defined-outside-init
connection.database = "-"
connection2 = dbapi.instrument_connection(self.tracer, connection, "-")
self.assertIs(connection2._connection, connection)
self.assertIs(connection2.__wrapped__, connection)

connection3 = dbapi.uninstrument_connection(connection2)
self.assertIs(connection3, connection)
Expand All @@ -372,12 +370,10 @@ def mock_connect(*args, **kwargs):
server_host = kwargs.get("server_host")
server_port = kwargs.get("server_port")
user = kwargs.get("user")
return MockConnectionWithAttributes(
database, server_port, server_host, user
)
return MockConnection(database, server_port, server_host, user)


class MockConnectionWithAttributes:
class MockConnection:
def __init__(self, database, server_port, server_host, user):
self.database = database
self.server_port = server_port
Expand Down Expand Up @@ -410,7 +406,3 @@ def executemany(self, query, params=None, throw_exception=False):
def callproc(self, query, params=None, throw_exception=False):
if throw_exception:
raise Exception("Test Exception")


class MockConnectionEmpty:
pass
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from django.test import SimpleTestCase
from django.test.utils import setup_test_environment, teardown_test_environment

from opentelemetry import trace as trace_api
from opentelemetry.instrumentation.django import (
DjangoInstrumentor,
_DjangoMiddleware,
Expand Down Expand Up @@ -424,6 +425,16 @@ async def test_tracer_provider_traced(self):
span.resource.attributes["resource-key"], "resource-value"
)

async def test_no_op_tracer_provider(self):
_django_instrumentor.uninstrument()
_django_instrumentor.instrument(
tracer_provider=trace_api.NoOpTracerProvider()
)

await self.async_client.post("/traced/")
spans = self.exporter.get_finished_spans()
self.assertEqual(len(spans), 0)


@patch.dict(
"os.environ",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from werkzeug.test import Client
from werkzeug.wrappers import Response

from opentelemetry import trace as trace_api
from opentelemetry.instrumentation.flask import FlaskInstrumentor
from opentelemetry.test.wsgitestutil import WsgiTestBase

Expand Down Expand Up @@ -78,3 +79,18 @@ def test_exluded_urls_explicit(self):
self.assertEqual([b"Hello: 456"], list(resp.response))
span_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(span_list), 1)

def test_no_op_tracer_provider(self):
FlaskInstrumentor().uninstrument()
FlaskInstrumentor().instrument(
tracer_provider=trace_api.NoOpTracerProvider()
)

self.app = flask.Flask(__name__)
self.app.route("/hello/<int:helloid>")(self._hello_endpoint)
# pylint: disable=attribute-defined-outside-init
self.client = Client(self.app, Response)
self.client.get("/hello/123")

span_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(span_list), 0)
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from unittest.mock import Mock, patch
from unittest import mock

import mysql.connector

Expand All @@ -23,15 +23,6 @@
from opentelemetry.test.test_base import TestBase


def mock_connect(*args, **kwargs):
class MockConnection:
def cursor(self):
# pylint: disable=no-self-use
return Mock()

return MockConnection()


def connect_and_execute_query():
cnx = mysql.connector.connect(database="test")
cursor = cnx.cursor()
Expand All @@ -47,9 +38,9 @@ def tearDown(self):
with self.disable_logging():
MySQLInstrumentor().uninstrument()

@patch("mysql.connector.connect", new=mock_connect)
@mock.patch("mysql.connector.connect")
# pylint: disable=unused-argument
def test_instrumentor(self):
def test_instrumentor(self, mock_connect):
MySQLInstrumentor().instrument()

connect_and_execute_query()
Expand All @@ -71,8 +62,9 @@ def test_instrumentor(self):
spans_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans_list), 1)

@patch("mysql.connector.connect", new=mock_connect)
def test_custom_tracer_provider(self):
@mock.patch("mysql.connector.connect")
# pylint: disable=unused-argument
def test_custom_tracer_provider(self, mock_connect):
resource = resources.Resource.create({})
result = self.create_tracer_provider(resource=resource)
tracer_provider, exporter = result
Expand All @@ -86,9 +78,9 @@ def test_custom_tracer_provider(self):

self.assertIs(span.resource, resource)

@patch("mysql.connector.connect", new=mock_connect)
@mock.patch("mysql.connector.connect")
# pylint: disable=unused-argument
def test_instrument_connection(self):
def test_instrument_connection(self, mock_connect):
cnx, query = connect_and_execute_query()

spans_list = self.memory_exporter.get_finished_spans()
Expand All @@ -101,18 +93,18 @@ def test_instrument_connection(self):
spans_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans_list), 1)

@patch("mysql.connector.connect", new=mock_connect)
def test_instrument_connection_no_op_tracer_provider(self):
@mock.patch("mysql.connector.connect")
def test_instrument_connection_no_op_tracer_provider(self, mock_connect):
tracer_provider = trace_api.NoOpTracerProvider()
MySQLInstrumentor().instrument(tracer_provider=tracer_provider)
connect_and_execute_query()

spans_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans_list), 0)

@patch("mysql.connector.connect", new=mock_connect)
@mock.patch("mysql.connector.connect")
# pylint: disable=unused-argument
def test_uninstrument_connection(self):
def test_uninstrument_connection(self, mock_connect):
MySQLInstrumentor().instrument()
cnx, query = connect_and_execute_query()

Expand Down
Loading

0 comments on commit ffed02e

Please sign in to comment.