Skip to content

Cache and expose extractors #155

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nuno-andre
Copy link

This PR caches and exposes the extractors dict for two purposes:

  • Loads the dict only during initialization, instead of on every logger invocation.
  • Provides a clearer and more scalable way to subclass StdlibFormatter.

This is an adapted use case from the library we use for several services in our company that adds information about errors in HTTP requests :

from typing import TYPE_CHECKING
from functools import lru_cache

from ecs_logging import StdlibFormatter
import httpx

if TYPE_CHECKING:
    from logging import LogRecord


httpx._utils.SENSITIVE_HEADERS.update({
    'apikey', 'api-key', 'x-api-key', 'x-auth-token',
})


class EcsFormatter(StdlibFormatter):

    @property
    @lru_cache
    def extractors(self):
        extractors = super().extractors
        extractors['http'] = self._extract_httpx_response
        return extractors

    def _extract_httpx_response(self, record: 'LogRecord') -> dict | None:
        if not record.exc_info or not isinstance(record.exc_info[1], httpx.HTTPStatusError):
            return None

        resp = record.exc_info[1].response

        redact = lambda x: dict(httpx._utils.obfuscate_sensitive_headers(x.multi_items()))

        if (url := resp.request.url).password:
            url = str(url).replace(f':{url.password}@', ':[secure]@')

        return {
            'version':          resp.http_version,
            'request':  {
                'method':       resp.request.method,
                'url':          str(url),
                'headers':      redact(resp.request.headers),
                'body.content': resp.request.content,
            },
            'response': {
                'status_code':  resp.status_code,
                'headers':      redact(resp.headers),
                'body.content': resp.content,
            }
        }

Additionally, this PR also:

@@ -1,16 +1,16 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.0.1
rev: v5.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Please drop these changes, we can do them in another PR

(or remove fields from) the JSON before being dumped into a string.
@property
@lru_cache
def extractors(self) -> Dict[str, Callable[[logging.LogRecord], Any]]:
Copy link
Member

Choose a reason for hiding this comment

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

I'm not very inclined to add a new public property, no objection in creating a private _extractors though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants