Add include/exclude settings for Hybrid Agent tracers (#1681)

* Add include/exclude settings for tracers

* Fix typos

* Remove duplicate test case

* [MegaLinter] Apply linters fixes

* [MegaLinter] Apply linters fixes

* Remove redundant ENV VAR logic

* [MegaLinter] Apply linters fixes

* Safeguard explicit setting of exclude/include to None

* Remove redundant checks

* Comments to clarify settings used

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Author: lrafeei

newrelic/api/opentelemetry.py

@@ -14,7 +14,6 @@
 
 import json
 import logging
-import os
 import sys
 import time
 from contextlib import contextmanager
@@ -46,6 +45,7 @@
 from newrelic.api.transaction import Sentinel, current_transaction
 from newrelic.api.web_transaction import WebTransaction, WSGIWebTransaction
 from newrelic.core.attribute import sanitize
+from newrelic.core.config import global_settings
 from newrelic.core.database_utils import (
     _all_literals_re,
     _quotes_table,
@@ -175,6 +175,9 @@ def __init__(
             # create another transaction or trace, but rather just
             # append existing attributes to the existing transaction.
             self.nr_trace = current_nr_trace
+            if not self.nr_trace:
+                return
+
             # Add Instrumentation Scope Attributes
             self.nr_trace._add_agent_attribute("otel.scope.name", self.attributes.get("library_name"))
             self.nr_trace._add_agent_attribute("otel.scope.version", self.attributes.get("library_version"))
@@ -658,6 +661,7 @@ def __init__(
         self.schema_url = schema_url
         self.tracer_attributes = attributes or {}
         self.resource = resource
+        self.exclude_tracer = False
 
     def _create_web_transaction(self, nr_headers=None):
         if "nr.wsgi.environ" in self.attributes:
@@ -739,12 +743,17 @@ def start_span(
 
         self._record_exception = record_exception
         self.set_status_on_exception = set_status_on_exception
+        self.settings = getattr(self.nr_application, "settings", None) or global_settings()
 
-        if not (
-            self.nr_application.settings and self.nr_application.settings.opentelemetry.enabled
-        ) and not os.environ.get("NEW_RELIC_OPENTELEMETRY_ENABLED"):
+        if self.settings and not self.settings.opentelemetry.enabled:
             return otel_api_trace.INVALID_SPAN
 
+        if self.settings and (
+            not self.settings.opentelemetry.traces.enabled
+            or (self.instrumentation_library in self.settings.opentelemetry.traces.exclude)
+        ):
+            self.exclude_tracer = True
+
         # Retrieve parent span
         parent_span_context = otel_api_trace.get_current_span(context).get_span_context()
 
@@ -757,7 +766,7 @@ def start_span(
 
         parent_span_trace_id = None
         nr_headers = {}
-        if parent_span_context and self.nr_application.settings.distributed_tracing.enabled:
+        if parent_span_context and self.settings.distributed_tracing.enabled:
             parent_span_trace_id = parent_span_context.trace_id
             if len(parent_span_context.trace_state) > 0:
                 # If headers did not propagate from an existing transaction due
@@ -770,16 +779,14 @@ def start_span(
                     f"00-{parent_span_trace_id:032x}-{parent_span_span_id:016x}-{'01' if parent_span_trace_flag else '00'}"
                 )
 
-        if not self.nr_application.settings.opentelemetry.traces.enabled:
+        if self.exclude_tracer:
             create_nr_trace = False
 
         # If remote_parent, transaction must be created, regardless of kind type
         # Make sure we transfer DT headers when we are here, if DT is enabled
-        if parent_span_context and parent_span_context.is_remote:
+        if parent_span_context and parent_span_context.is_remote and not self.exclude_tracer:
             if kind in (otel_api_trace.SpanKind.SERVER, otel_api_trace.SpanKind.CLIENT):
                 transaction = self._create_web_transaction(nr_headers)
-                if not self.nr_application.settings.opentelemetry.traces.enabled:
-                    transaction.ignore_transaction = True
                 transaction.__enter__()
                 # If a transaction was already active, we want to create
                 # an NR trace under the existing transaction.  Otherwise,
@@ -789,8 +796,6 @@ def start_span(
                     create_nr_trace = False
             elif kind in (otel_api_trace.SpanKind.PRODUCER, otel_api_trace.SpanKind.INTERNAL):
                 transaction = BackgroundTask(self.nr_application, name=self.name)
-                if not self.nr_application.settings.opentelemetry.traces.enabled:
-                    transaction.ignore_transaction = True
                 transaction.__enter__()
                 # If a transaction was already active, we want to create
                 # an NR trace under the existing transaction.  Otherwise,
@@ -806,8 +811,6 @@ def start_span(
                     application=self.nr_application,
                     headers=nr_headers,
                 )
-                if not self.nr_application.settings.opentelemetry.traces.enabled:
-                    transaction.ignore_transaction = True
                 transaction.__enter__()
                 # In the case of a Kafka consumer span, we do not want to create
                 # a trace regardless of whether a transaction already existed.
@@ -831,14 +834,12 @@ def start_span(
             if kind == otel_api_trace.SpanKind.SERVER:
                 if transaction:
                     nr_trace_type = FunctionTrace
-                elif not transaction:
+                elif not transaction and not self.exclude_tracer:
                     transaction = self._create_web_transaction(nr_headers)
 
                     transaction._trace_id = (
                         f"{parent_span_trace_id:x}" if parent_span_trace_id else transaction.trace_id
                     )
-                    if not self.nr_application.settings.opentelemetry.traces.enabled:
-                        transaction.ignore_transaction = True
                     transaction.__enter__()
                     create_nr_trace = False
             elif kind == otel_api_trace.SpanKind.INTERNAL:
@@ -869,16 +870,14 @@ def start_span(
                     # Note that for Kafka, this flag will not be
                     # set, so we will not create a MessageTrace
                     nr_trace_type = MessageTrace
-                else:
+                elif not self.exclude_tracer:
                     transaction = MessageTransaction(
                         library=self.instrumentation_library,
                         destination_type="Exchange",
                         destination_name=self.name,
                         application=self.nr_application,
                         headers=nr_headers,
                     )
-                    if not self.nr_application.settings.opentelemetry.traces.enabled:
-                        transaction.ignore_transaction = True
                     transaction.__enter__()
                     # In the case of a Kafka consumer span, we do not want to create
                     # a trace regardless of whether a transaction already existed.

newrelic/common/opentelemetry_tracers.py

@@ -17,8 +17,27 @@
 # OpenTelemetry instrumentor and the list of targets in the
 # NR hooks that need to be disabled if the OpenTelemetry
 # instrumentor is to be used instead.
-HYBRID_AGENT_DEFAULT_INCLUDED_TRACERS_TO_NR_HOOKS = {
+OPENTELEMETRY_ONLY_TRACERS_TO_NR_HOOKS = {
     "aio-pika": [],
+    "aiopg": [
+        "psycopg2",
+        "psycopg2._psycopg2",
+        "psycopg2.extensions",
+        "psycopg2._json",
+        "psycopg2._range",
+        "psycopg2.sql",
+    ],
+    "aiokafka": [],
+    "asyncclick": [],
+    "click": [],
+    "remoulade": [],
+    "sqlalchemy": [],
+    "system_metrics": [],
+    "threading": [],
+    "tortoiseorm": [],
+}
+
+NEW_RELIC_AND_OPENTELEMETRY_TRACERS_TO_NR_HOOKS = {
     "aiohttp_client": ["aiohttp.client", "aiohttp.client_reqrep"],
     "aiohttp_server": [
         "aiohttp.web",
@@ -28,15 +47,6 @@
         "aiohttp.web_urldispatcher",
         "aiohttp.protocol",
     ],
-    "aiokafka": [],
-    "aiopg": [
-        "psycopg2",
-        "psycopg2._psycopg2",
-        "psycopg2.extensions",
-        "psycopg2._json",
-        "psycopg2._range",
-        "psycopg2.sql",
-    ],
     "ariadne": [
         "ariadne.graphql",
         "graphql.graphql",
@@ -50,14 +60,12 @@
         "graphql.validation.validation",
         "graphql.type.schema",
     ],
-    "asyncclick": [],
     "asyncpg": ["asyncpg.connect_utils", "asyncpg.protocol"],
     # "boto": [], # this is boto3
     # "boto3": [],    # this is boto3sqs
     # "botocore": [],
     "cassandra": ["cassandra", "cassandra.cluster"],
     "celery": ["celery.local", "celery.app.trace", "celery.worker", "celery.concurrency.prefork", "billiard.pool"],
-    "click": [],
     "confluent_kafka": [
         "confluent_kafka.cimpl",
         "confluent_kafka.serializing_producer",
@@ -179,7 +187,6 @@
         "redis.commands.graph.commands",
         "redis.commands.vectorset.commands",
     ],
-    "remoulade": [],
     "requests": [
         "requests.sessions",
         "requests.api",
@@ -189,7 +196,6 @@
         "http.client",
         "httplib2",
     ],
-    "sqlalchemy": [],
     "sqlite3": ["sqlite3", "sqlite3.dbapi2", "pysqlite2", "pysqlite2.dbapi2"],
     "starlette": [
         "starlette.requests",
@@ -215,8 +221,6 @@
         "graphql.validation.validation",
         "graphql.type.schema",
     ],
-    "system_metrics": [],
-    "threading": [],
     "tornado": [
         "tornado.httpserver",
         "tornado.httputil",
@@ -225,11 +229,14 @@
         "tornado.web",
         "http.client",
     ],
-    "tortoiseorm": [],
     "urllib": ["urllib.request", "http.client"],
     "urllib3": ["urllib3.connectionpool", "urllib3.connection", "requests.packages.urllib3.connection", "http.client"],
 }
 
+ALL_LIBRARY_TRACERS_TO_NR_HOOKS = {
+    **NEW_RELIC_AND_OPENTELEMETRY_TRACERS_TO_NR_HOOKS,
+    **OPENTELEMETRY_ONLY_TRACERS_TO_NR_HOOKS,
+}
 
 TEMPORARILY_DISABLED_OPENTELEMETRY_FRAMEWORKS = {
     "boto",

newrelic/config.py

@@ -44,7 +44,8 @@
 from newrelic.common.log_file import initialize_logging
 from newrelic.common.object_names import callable_name, expand_builtin_exception_name
 from newrelic.common.opentelemetry_tracers import (
-    HYBRID_AGENT_DEFAULT_INCLUDED_TRACERS_TO_NR_HOOKS,
+    ALL_LIBRARY_TRACERS_TO_NR_HOOKS,
+    OPENTELEMETRY_ONLY_TRACERS_TO_NR_HOOKS,
     TEMPORARILY_DISABLED_OPENTELEMETRY_FRAMEWORKS,
 )
 from newrelic.common.package_version_utils import get_package_version
@@ -219,6 +220,10 @@ def _map_split_strings(s):
     return s.split()
 
 
+def _map_split_string_by_comma(s):
+    return newrelic.core.config.parse_comma_separated_into_set(s)
+
+
 def _map_console_listener_socket(s):
     return s % {"pid": os.getpid()}
 
@@ -680,6 +685,8 @@ def _process_configuration(section):
     _process_setting(section, "instrumentation.middleware.django.include", "get", _map_inc_excl_middleware)
     _process_setting(section, "opentelemetry.enabled", "getboolean", None)
     _process_setting(section, "opentelemetry.traces.enabled", "getboolean", None)
+    _process_setting(section, "opentelemetry.traces.exclude", "get", _map_split_string_by_comma)
+    _process_setting(section, "opentelemetry.traces.include", "get", _map_split_string_by_comma)
 
 
 # Loading of configuration from specified file and for specified
@@ -4456,14 +4463,35 @@ def _is_installed(req):
     return False
 
 
-def _process_opentelemetry_instrumentation_entry_points(
-    final_include_dict=HYBRID_AGENT_DEFAULT_INCLUDED_TRACERS_TO_NR_HOOKS,
-):
-    from importlib.metadata import entry_points
+def _tracer_include_and_exclude_filter():
+    """Uses the values in `opentelemetry.traces.include` and
+    `opentelemetry.traces.exclude` settings, along with the
+    internal included defaults, to determine which tracers
+    should be used.
+    """
+
+    user_exclude = _settings.opentelemetry.traces.exclude or newrelic.core.config._environ_as_comma_separated_set(
+        "NEW_RELIC_OPENTELEMETRY_TRACES_EXCLUDE"
+    )
+    user_include = _settings.opentelemetry.traces.include or newrelic.core.config._environ_as_comma_separated_set(
+        "NEW_RELIC_OPENTELEMETRY_TRACES_INCLUDE"
+    )
 
+    tracer_include_union = {*OPENTELEMETRY_ONLY_TRACERS_TO_NR_HOOKS.keys(), *user_include}
+    mask = tracer_include_union & user_exclude
+    final_include_set = tracer_include_union ^ mask
+
+    return final_include_set
+
+
+def _process_opentelemetry_instrumentation_entry_points():
     if not _settings.opentelemetry.enabled or not _is_installed("opentelemetry-api"):
         return
 
+    include_set = _tracer_include_and_exclude_filter()
+
+    from importlib.metadata import entry_points
+
     group = "opentelemetry_instrumentor"
 
     try:
@@ -4476,27 +4504,19 @@ def _process_opentelemetry_instrumentation_entry_points(
     entry_points_generator = (
         entrypoint
         for entrypoint in _entry_points
-        if entrypoint.name in final_include_dict
-        and entrypoint.name not in TEMPORARILY_DISABLED_OPENTELEMETRY_FRAMEWORKS
+        if entrypoint.name in include_set and entrypoint.name not in TEMPORARILY_DISABLED_OPENTELEMETRY_FRAMEWORKS
     )
 
     for entrypoint in entry_points_generator:
         opentelemetry_entrypoints.append(entrypoint)
-        opentelemetry_instrumentation.extend(final_include_dict[entrypoint.name])
+        opentelemetry_instrumentation.extend(ALL_LIBRARY_TRACERS_TO_NR_HOOKS[entrypoint.name])
 
     # Check for native installations
-    # NOTE: This logic will change once enabled and disabled
-    # functionality is implemented for opentelemetry.traces setting.
-    # NOTE: elasticsearch is instrumented both with libs and natively.
-    # To handle this case: If lib is installed, the library itself
-    # will check for native instrumentation and switch to that on its
-    # own, but if not, native instrumentation could still be used and
-    # we would not know.  We handle this as we are with strawberry-graphql
-    # and ariadne where we check to see if opentelemetry-api and the
-    # specific library are installed on the system.
+    # NOTE: for native instrumentation to work, the tracer name must be
+    # explicitly included in the opentelemetry.traces.include setting.
     for lib in ["strawberry-graphql", "ariadne", "elasticsearch"]:
-        if _is_installed(lib):
-            opentelemetry_instrumentation.extend(final_include_dict[lib])
+        if _is_installed(lib) and (lib in include_set):
+            opentelemetry_instrumentation.extend(ALL_LIBRARY_TRACERS_TO_NR_HOOKS[lib])
 
 
 def _process_opentelemetry_instrumentors():

newrelic/core/config.py

@@ -72,6 +72,13 @@ def emit(self, record):
 _logger.addHandler(_NullHandler())
 
 
+def parse_comma_separated_into_set(string):
+    if not string:
+        return set()
+    # Strip the string in case user has separated by string and space
+    return {item.strip() for item in string.split(",")}
+
+
 def parse_space_separated_into_list(string):
     return string.split()
 
@@ -753,6 +760,11 @@ def _environ_as_set(name, default=""):
     return set(value.split())
 
 
+def _environ_as_comma_separated_set(name, default=""):
+    value = os.environ.get(name, default)
+    return parse_comma_separated_into_set(value)
+
+
 def _environ_as_mapping(name, default=""):
     result = []
     items = os.environ.get(name, default)
@@ -1409,6 +1421,12 @@ def default_otlp_host(host):
 _settings.ml_insights_events.enabled = _environ_as_bool("NEW_RELIC_ML_INSIGHTS_EVENTS_ENABLED", default=False)
 _settings.opentelemetry.enabled = _environ_as_bool("NEW_RELIC_OPENTELEMETRY_ENABLED", default=False)
 _settings.opentelemetry.traces.enabled = _environ_as_bool("NEW_RELIC_OPENTELEMETRY_TRACES_ENABLED", default=True)
+_settings.opentelemetry.traces.exclude = _environ_as_comma_separated_set(
+    "NEW_RELIC_OPENTELEMETRY_TRACES_EXCLUDE", default=""
+)
+_settings.opentelemetry.traces.include = _environ_as_comma_separated_set(
+    "NEW_RELIC_OPENTELEMETRY_TRACES_INCLUDE", default=""
+)
 
 
 def global_settings():