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

Make Client object globally available #1043

Merged
merged 6 commits into from
Mar 1, 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
2 changes: 2 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ endif::[]
[float]
===== Features

* Add global access to Client singleton object at `elasticapm.get_client()` {pull}1043[#1043]


[float]
===== Bug fixes
Expand Down
2 changes: 1 addition & 1 deletion elasticapm/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
import sys

from elasticapm.base import Client
from elasticapm.base import Client, get_client # noqa: F401
from elasticapm.conf import setup_logging # noqa: F401
from elasticapm.instrumentation.control import instrument, uninstrument # noqa: F401
from elasticapm.traces import ( # noqa: F401
Expand Down
19 changes: 19 additions & 0 deletions elasticapm/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@

__all__ = ("Client",)

CLIENT_SINGLETON = None


class Client(object):
"""
Expand Down Expand Up @@ -205,6 +207,9 @@ def __init__(self, config=None, **inline):
if config.enabled:
self.start_threads()

# Save this Client object as the global CLIENT_SINGLETON
set_client(self)

def start_threads(self):
with self._thread_starter_lock:
current_pid = os.getpid()
Expand Down Expand Up @@ -298,6 +303,8 @@ def close(self):
with self._thread_starter_lock:
for _, manager in self._thread_managers.items():
manager.stop_thread()
global CLIENT_SINGLETON
CLIENT_SINGLETON = None

def get_service_info(self):
if self._service_info:
Expand Down Expand Up @@ -611,3 +618,15 @@ class DummyClient(Client):

def send(self, url, **kwargs):
return None


def get_client():
return CLIENT_SINGLETON


def set_client(client):
global CLIENT_SINGLETON
if CLIENT_SINGLETON:
logger = get_logger("elasticapm")
logger.debug("Client object is being set more than once")
CLIENT_SINGLETON = client
3 changes: 0 additions & 3 deletions elasticapm/contrib/aiohttp/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
import elasticapm
from elasticapm import Client

CLIENT_KEY = "_elasticapm_client_instance"


class ElasticAPM:
def __init__(self, app, client=None):
Expand All @@ -43,7 +41,6 @@ def __init__(self, app, client=None):
config.setdefault("framework_name", "aiohttp")
config.setdefault("framework_version", aiohttp.__version__)
client = Client(config=config)
app[CLIENT_KEY] = client
self.app = app
self.client = client
self.install_tracing(app, client)
Expand Down
6 changes: 2 additions & 4 deletions elasticapm/contrib/aiohttp/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from aiohttp.web import HTTPException, Response, middleware

import elasticapm
from elasticapm import get_client
from elasticapm.conf import constants
from elasticapm.contrib.aiohttp.utils import get_data_from_request, get_data_from_response
from elasticapm.utils.disttracing import TraceParent
Expand All @@ -44,13 +45,10 @@ def merge_duplicate_headers(cls, headers, key):


def tracing_middleware(app):
from elasticapm.contrib.aiohttp import CLIENT_KEY # noqa

async def handle_request(request, handler):
elasticapm_client = app.get(CLIENT_KEY)
elasticapm_client = get_client()
should_trace = elasticapm_client and not elasticapm_client.should_ignore_url(request.path)
if should_trace:
request[CLIENT_KEY] = elasticapm_client
trace_parent = AioHttpTraceParent.from_headers(request.headers)
elasticapm_client.begin_transaction("request", trace_parent=trace_parent)
resource = request.match_info.route.resource
Expand Down
28 changes: 12 additions & 16 deletions elasticapm/contrib/django/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from django.db import DatabaseError
from django.http import HttpRequest

from elasticapm import get_client as _get_client
from elasticapm.base import Client
from elasticapm.conf import constants
from elasticapm.contrib.django.utils import iterate_with_template_sources
Expand All @@ -49,31 +50,26 @@


default_client_class = "elasticapm.contrib.django.DjangoClient"
_client = (None, None)


def get_client(client=None):
def get_client():
"""
Get an ElasticAPM client.

:param client:
:return:
:rtype: elasticapm.base.Client
"""
global _client

tmp_client = client is not None
if not tmp_client:
config = getattr(django_settings, "ELASTIC_APM", {})
client = config.get("CLIENT", default_client_class)

if _client[0] != client:
client_class = import_string(client)
instance = client_class()
if not tmp_client:
_client = (client, instance)
return instance
return _client[1]
if _get_client():
return _get_client()

config = getattr(django_settings, "ELASTIC_APM", {})
client = config.get("CLIENT", default_client_class)
client_class = import_string(client)
instance = client_class()
# `instance` will already be in elasticapm.base.CLIENT_SINGLETON due to the
# `__init__()` for Client
return instance


class DjangoClient(Client):
Expand Down
26 changes: 11 additions & 15 deletions elasticapm/contrib/flask/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

import elasticapm
import elasticapm.instrumentation.control
from elasticapm import get_client
from elasticapm.base import Client
from elasticapm.conf import constants, setup_logging
from elasticapm.contrib.flask.utils import get_data_from_request, get_data_from_response
Expand All @@ -50,17 +51,6 @@
logger = get_logger("elasticapm.errors.client")


def make_client(client_cls, app, **defaults):
config = app.config.get("ELASTIC_APM", {})

if "framework_name" not in defaults:
defaults["framework_name"] = "flask"
defaults["framework_version"] = getattr(flask, "__version__", "<0.7")

client = client_cls(config, **defaults)
return client


class ElasticAPM(object):
"""
Flask application for Elastic APM.
Expand Down Expand Up @@ -97,10 +87,18 @@ class ElasticAPM(object):
def __init__(self, app=None, client=None, client_cls=Client, logging=False, **defaults):
self.app = app
self.logging = logging
self.client_cls = client_cls
self.client = client
self.client = client or get_client()

if app:
if not self.client:
config = app.config.get("ELASTIC_APM", {})

if "framework_name" not in defaults:
defaults["framework_name"] = "flask"
defaults["framework_version"] = getattr(flask, "__version__", "<0.7")

self.client = client_cls(config, **defaults)

self.init_app(app, **defaults)

def handle_exception(self, *args, **kwargs):
Expand All @@ -125,8 +123,6 @@ def handle_exception(self, *args, **kwargs):

def init_app(self, app, **defaults):
self.app = app
if not self.client:
self.client = make_client(self.client_cls, app, **defaults)

# 0 is a valid log level (NOTSET), so we need to check explicitly for it
if self.logging or self.logging is logging.NOTSET:
Expand Down
6 changes: 2 additions & 4 deletions elasticapm/contrib/opentracing/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,15 @@
from opentracing.tracer import Tracer as TracerBase

import elasticapm
from elasticapm import instrument, traces
from elasticapm import get_client, instrument, traces
from elasticapm.conf import constants
from elasticapm.contrib.opentracing.span import OTSpan, OTSpanContext
from elasticapm.utils import compat, disttracing


class Tracer(TracerBase):
_elasticapm_client_class = elasticapm.Client

def __init__(self, client_instance=None, config=None, scope_manager=None):
self._agent = client_instance or self._elasticapm_client_class(config=config)
self._agent = client_instance or get_client() or elasticapm.Client(config=config)
if scope_manager and not isinstance(scope_manager, ThreadLocalScopeManager):
warnings.warn(
"Currently, the Elastic APM opentracing bridge only supports the ThreadLocalScopeManager. "
Expand Down
3 changes: 2 additions & 1 deletion elasticapm/contrib/paste.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.


from elasticapm import get_client
from elasticapm.base import Client
from elasticapm.middleware import ElasticAPM


def filter_factory(app, global_conf, **kwargs):
client = Client(**kwargs)
client = get_client() or Client(**kwargs)
return ElasticAPM(app, client)
3 changes: 2 additions & 1 deletion elasticapm/contrib/pylons/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE


from elasticapm import get_client
from elasticapm.base import Client
from elasticapm.middleware import ElasticAPM as Middleware
from elasticapm.utils import compat
Expand All @@ -44,5 +45,5 @@ def list_from_setting(config, setting):
class ElasticAPM(Middleware):
def __init__(self, app, config, client_cls=Client):
client_config = {key[11:]: val for key, val in compat.iteritems(config) if key.startswith("elasticapm.")}
client = client_cls(**client_config)
client = get_client() or client_cls(**client_config)
super(ElasticAPM, self).__init__(app, client)
11 changes: 0 additions & 11 deletions tests/contrib/django/django_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,17 +511,6 @@ def test_response_error_id_middleware(django_elasticapm_client, client):
assert event["id"] == headers["X-ElasticAPM-ErrorId"]


def test_get_client(django_elasticapm_client):
with mock.patch.dict("os.environ", {"ELASTIC_APM_METRICS_INTERVAL": "0ms"}):
client2 = get_client("elasticapm.base.Client")
try:
assert get_client() is get_client()
assert client2.__class__ == Client
finally:
get_client().close()
client2.close()


Comment on lines -514 to -524
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Django's get_client() is only ever called with an argument here, in this test. Thus, I don't think we need the type-checking functionality that was in the old implementation. I just removed the test.

@pytest.mark.parametrize("django_elasticapm_client", [{"capture_body": "errors"}], indirect=True)
def test_raw_post_data_partial_read(django_elasticapm_client):
v = compat.b('{"foo": "bar"}')
Expand Down