moleculerjs / moleculer-web

:earth_africa: Official API Gateway service for Moleculer framework
http://moleculer.services/docs/moleculer-web.html
MIT License
291 stars 118 forks source link

Capture HTTP details as tags for tracing #210

Closed edud69 closed 3 years ago

edud69 commented 3 years ago

The tracer is not returning HTTP details (status code, method, url) when used with a web path. I am currently testing with Datadog and Console exporters and it seems the HTTP tags are missing.

I would expect something similar to this: image

Thanks!

icebob commented 3 years ago

URL and method added to the span tags by default: https://github.com/moleculerjs/moleculer-web/blob/master/src/index.js#L114 But you can overwrite it in your service.

edud69 commented 3 years ago

URL and method added to the span tags by default: https://github.com/moleculerjs/moleculer-web/blob/master/src/index.js#L114 But you can overwrite it in your service.

Thanks for the quick reply!

I will look into it, it seems there is a bug where the globalActionTags is not fired when it is defined as a callback and the endpoint already has a local definition. Is this the wanted behavior?

from tracing.js

    // local action tags take precedence
    if (isFunction(opts.tags)) {
        actionTags = opts.tags;
    } else if (!opts.tags && isFunction(globalActionTags)) {
        actionTags = globalActionTags;
    } else {
        // By default all params are captured. This can be overridden globally and locally
        actionTags = { ...{ params: true }, ...globalActionTags, ...opts.tags };  <<< here if globalActionTags is a function, it is not firing the callback
    }

Also, is there something I can use to retrieve the HTTP Response in the tag callback?

kthompson23 commented 3 years ago

Before I added the global action/event tags the existing behavior for local action/event tags was that you could either have tags using the options object or a call back.

if (isFunction(opts.tags)) {
    const res = opts.tags.call(ctx.service, ctx);
    if (res)
        Object.assign(tags, res);

} else if (isPlainObject(opts.tags)) {
    if (opts.tags.params === true)
        tags.params = ctx.params != null && isPlainObject(ctx.params) ? Object.assign({}, ctx.params) : ctx.params;
    else if (Array.isArray(opts.tags.params))
        tags.params = _.pick(ctx.params, opts.tags.params);

    if (opts.tags.meta === true)
        tags.meta = ctx.meta != null ? Object.assign({}, ctx.meta) : ctx.meta;
    else if (Array.isArray(opts.tags.meta))
        tags.meta = _.pick(ctx.meta, opts.tags.meta);
}

So I went with the same restriction and came up with this hierarchy:

  1. only local callback
  2. only global callback
  3. merge of global and local tag objects

Another thing was that I wanted this change in 0.14.x so I didn't want to make any breaking change to the local action/event configuration. The one issue of supporting both a globally defined callback and locally defined tags and vice-versa that I see is that there's no way to disable the globally defined callback or tag options. ie You define a global callback but for this one action you want just the locally defined tags to be applied. How do you prevent the globally defined callback from running?

edud69 commented 3 years ago

@kthompson23 Maybe we could provide a return value from the callback locally if we want to specify the overwrite of global tags? May not be the most elegant way to do it though.