feat: Add breadcrumb hints

[untitaker]

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.

[mitsuhiko]

@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]

@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]

Not urgent

[untitaker]

It's IMO wrong to call this function with something that can't be coerced into a dict. My personal opinion is that the error message of dict() is good enough in such cases

…

On Tue, Sep 4, 2018, 16:11 Mark Story ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In sentry_sdk/hub.py <#49 (comment)> : > @@ -161,6 +161,12 @@ def add_breadcrumb(self, *args, **kwargs): logger.info("Dropped breadcrumb because no client bound") return + hint = kwargs.pop("hint", None) + if not hint: + hint = {} + else: + hint = dict(hint) Do you need to catch errors here for when the argument cannot be converted into a dict? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#49 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAzHxd8msKvehgUStzCWmFoejpPpJA_Qks5uXonsgaJpZM4WYgBn> .

[untitaker]

@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]

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

[mitsuhiko]

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