Skip to content

feat: Add breadcrumb hints#49

Merged
untitaker merged 2 commits into
masterfrom
feat/breadcrumb-hints
Sep 5, 2018
Merged

feat: Add breadcrumb hints#49
untitaker merged 2 commits into
masterfrom
feat/breadcrumb-hints

Conversation

@untitaker

Copy link
Copy Markdown
Member

I wonder if we could just put the hint on the breadcrumb itself and pop it
before sending? I am unhappy that the hint argument for before_breadcrumb is
now mandatory.

@untitaker untitaker requested a review from mitsuhiko September 4, 2018 08:16
@mitsuhiko

Copy link
Copy Markdown
Contributor

@untitaker I made an experiment on the plane where i changed add_breadcrumb to this:

    def add_breadcrumb(self, *args, **kwargs):
        """Adds a breadcrumb."""
        client, scope = self._stack[-1]
        if client is None:
            logger.info("Dropped breadcrumb because no client bound")
            return

        if len(args) == 2:
            crumb, hint = args
        elif len(args) == 1:
            crumb = args[0] or {}
            hint = kwargs.pop("hint", None)
            crumb.update(kwargs)
        elif kwargs:
            hint = kwargs.pop("hint", None)
            crumb = kwargs

        if crumb is None:
            return

        if crumb.get("timestamp") is None:
            crumb["timestamp"] = datetime.utcnow()
        if crumb.get("type") is None:
            crumb["type"] = "default"

        original_crumb = crumb
        if client.options["before_breadcrumb"] is not None:
            crumb = client.options["before_breadcrumb"](crumb, hint)

        if crumb is not None:
            scope._breadcrumbs.append(crumb)
        else:
            logger.info("before breadcrumb dropped breadcrumb (%s)", original_crumb)
        while len(scope._breadcrumbs) >= client.options["max_breadcrumbs"]:
            scope._breadcrumbs.popleft()

That makes it less weird i think

@untitaker

Copy link
Copy Markdown
Member Author

@mitsuhiko I'd like to somehow stop parsing args/kwargs, not sure how yet. Is this PR urgent? I'd like to think about it for some more

@mitsuhiko

Copy link
Copy Markdown
Contributor

Not urgent

Comment thread sentry_sdk/hub.py Outdated
@untitaker

untitaker commented Sep 4, 2018 via email

Copy link
Copy Markdown
Member Author

@untitaker

Copy link
Copy Markdown
Member Author

@mitsuhiko Any opinions about:

def add_breadcrumb(self, crumb=None, hint=None, **kwargs):
    crumb = crumb or {}
    crumb.update(**kwargs)

Idk which cases I am changing by this.

@mitsuhiko

Copy link
Copy Markdown
Contributor

Might work? I guess at least. Seems fine by me.

@mitsuhiko

Copy link
Copy Markdown
Contributor

Maybe we wrap crumb or {} in a dict() for good measure.

@untitaker untitaker merged commit 79c7c7b into master Sep 5, 2018
@tonyo tonyo deleted the feat/breadcrumb-hints branch September 14, 2018 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants