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

WIP: Support for customizing the display of operation names #678

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bobrik
Copy link
Contributor

@bobrik bobrik commented Jan 15, 2021

See #675 for backstory. Marking it as WIP as it's far from ready.

With the following config file:

function UIConfig() {
    return {
        "traceProcessor": async function (trace) {
            for (const span of trace.spans) {
                const tags = {};

                for (const kv of span.tags) {
                    tags[kv.key] = kv.value;
                }

                if ("http.method" in tags) {
                    span.operationLabel = `${tags["http.method"]}${tags["http.status_code"]}`;
                }
            }
        },
    };
}

I can get the following view:

image

This doesn't change the stats view, as expected:

image

This solves the immediate issue, but I'd like to be able to do async fetches in my traceProcessor to allow augmenting traces with data that is not immediately available. One use-case is having a customer specific SLO and adding a warning to spans if they breach the SLO. Given that the call chain leading to traceProcessor is not async, this currently doesn't work. I'm not a front-end expert to figure this out by myself and it would be nice to get some hints.

Let me know if this is the right approach.

We use it to show in SpanBarRow to allow surfacing potentially high cardinality
data from tags right next to operation name where it's immediately visible.
If `traceProcessor` function is defined in the config, it acts
as a filter for traces before they are usable in UI.
@colelawrence
Copy link

I don't understand why this is described as "far from ready" the code is simple enough. I think it might be a good idea to clone the span into a separate data structure and use something akin to try { transformerFn(spanCopy) } catch (err) {} in case the internal structure of the span changes (we don't want to corrupt the UI with screwed up data if there is a mapping error).

I would also consider naming this API as unstableTraceTransformer(trace) until there's a data model that can be stabilized. I think it would also suffice as well to skip documentation to start getting feedback.

@yurishkuro
Copy link
Member

I think OP listed some of the reasons why it's "not ready", one is a lack of async behavior.

I also don't like such a brute force "transform trace" backdoor, it's dangerous. The specific behavior here is defining a label for operation name, which is a substantially more narrow use case than what transform-trace allows. Instead of having such UDF that can mutate the trace, and considering that the UI still needs to have knowledge about the label field, I would rather have a smaller scoped hook like computeOperationNameLabel(span, trace) -> string (and maybe it can be made async, this is a bit beyond my React knowledge).

The thing I do like about a code-based hook is that it unblocks this use case for motivated users, instead of waiting for us to decide to implement more configuration-driven approaches that were discussed in the ticket.

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