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

Request Tracking #6323

Open
yog27ray opened this issue Jan 8, 2020 · 13 comments
Open

Request Tracking #6323

yog27ray opened this issue Jan 8, 2020 · 13 comments
Labels
type:feature New feature or improvement of existing feature

Comments

@yog27ray
Copy link
Contributor

yog27ray commented Jan 8, 2020

I am unable to track down the main start point of any api request made to parse-server.
For Example:

Parse.Cloud.define('A', async (req, option) => {
    console.log('>>>>CallToA', option);
    return new Parse.Query(Parse.User).findAll(option);
});

Parse.Cloud.define('B', async (req, option) => {
    console.log('>>>>CallToB', option);
    return Parse.Cloud.run('A', {}, option);
});

Parse.Cloud.define('C', async (req, option) => {
    console.log('>>>>CallToC', option);
    return Parse.Cloud.run('A', {}, option);
});

If I get some error while making query in User table defined Cloud function "A". I am unable to trace back wheather client started requested from Cloud function "A" or "B" or "C".

What I am looking is an option to pass requestId that I can trace back. Like I can pass sessionToken if I can also pass requestId along with the request options passed during Parse.Cloud.run or ParseQuery.find or ParseQuery.findAll or ParseObject.save. I can just log the requestId from option to trace back source of origin.

@yog27ray yog27ray changed the title Pass "Request Id" in api calls Request Tracking Jan 8, 2020
@omairvaiyani
Copy link
Contributor

I believe this is currently not possible. Whilst you can use a middleware to set a request id like so:

import cuid from 'cuid';
const generateRequestId = () => cuid();

const app = express()
app.use((req, res, next) => {
   req.requestId = generateRequestId()
   next();
});
app.use(new ParseServer({ ...options }));

Unfortunately you cannot access it from within the cloud functions. We'd be able to consider a PR - this section is roughly where a change would be made. We need to consider what key should be used for the requestId - therefore, this should be a discussion before any PR is implemented.

@omairvaiyani omairvaiyani added discussion type:feature New feature or improvement of existing feature labels Jan 8, 2020
@yog27ray
Copy link
Contributor Author

yog27ray commented Jan 9, 2020

Thanks @omairvaiyani for considering it. As a hack I can get the generated requestId in cloud function also with the below code

import cuid from 'cuid';
const generateRequestId = () => cuid();

const app = express()
app.use((req, res, next) => {
   req.headers.requestId = generateRequestId()
   next();
});
app.use(new ParseServer({ ...options }));
Parse.Cloud.define('A', async (req, option) => {
    console.log('>>>>RequestId', req.headers.requestId);
    return new Parse.Query(Parse.User).findAll(option);
});

But the problem is not just getting requestId in cloud function but to forward that requestId in subsequent parse calls.

@omairvaiyani
Copy link
Contributor

Whilst this is not a direct fix for your problem, I tend to see the use of inter-server cloud function calls as an architectural problem. Ideally, as your codebase grows, you should aim to only use Parse.Cloud.define to expose your business logic to your client API consumers - but internally, you should execute other functions directly. See this very oversimplified example:

const doA = async (params, options) => {
   const doBResponse = await doB({ foo: params.foo }, options);
   return { outcome: doBResponse.outcome };
};

const doB = async (params, options) => {
   const outcome = await _internalFunc(params.foo, options.sessionToken)
   return { outcome };
};

const cloudFunctionWrapper = (func) => {
  return async (request) => {
    return await func(request.params, { sessionToken: request.user.getSessionToken()  })
  }
}

Parse.Cloud.define('doA',  cloudFunctionWrapper(doA));
Parse.Cloud.define('doA',  cloudFunctionWrapper(doB));

With this pattern, which can be infinitely further abstracted to suit your application's needs, can allow you to have a far greater degree of control in your application layer. If you're looking at request tracking, chances are you're scaling up - treat the Parse Cloud SDK as a support layer for your API, rather than the entire API itself.

@yog27ray
Copy link
Contributor Author

The above mentioned case was just an example, we don't actually call a cloud-function from another cloud-function. But if cloud-function saves some object then beforeSave and afterSave trigger doesn't have information about source of origin.

@arjun3396
Copy link
Contributor

is solving this issue also solves the problem with opentracing with parse-server as I need to have spans in every function because parse is not internally passing information about the function which made call to another function. I feel the feature I need is somehow related to what is reported by @yog27ray . If someone works on this feature can you also add some information related to which function made a call to another function. @omairvaiyani needs your views on it if this thing is feasible or not?

@omairvaiyani
Copy link
Contributor

I would say it's feasible but needs a substantial RFC as it covers many areas, not just functions. If this is a big use-case for your project/organisation, I'm sure we'd be very open to a well thought out discussion around what information should be captured and transmitted between cloud function calls, save hooks, etc.

@yog27ray
Copy link
Contributor Author

yog27ray commented Mar 31, 2020

@omairvaiyani I don't have much information about the code base but as per my understanding in order to implement opentracing jaeger-client. I would suggest following changes In the system.

  1. Add Opentraceing config in ParseServer option

    {
        appId: 'appId',
       ...
        tracer: {
            jaeger:  {
               ... // jaeger option
               enable: true // enable flag to start or stop jaeger
           }
        }
     }
    
  2. Create Jaeger Tracing file

    const initTracer = require('jaeger-client').initTracer;
    let tracer;
    
    exports.getJaegerTracer = getJaegerTracer;
    
    function getJaegerTracer(config) {
        if (!tracer && config && config.enable) {
            var options = { tags: { 'parse-server-version': 'version' } };
            tracer = initTracer(config, options);
        }
        return tracer;
    }
    
  3. create Opentraceing span from request via middleware.js.

    exports.handleOpentraceingHeaders = handleOpentraceingHeaders;
    ...
    const { FORMAT_HTTP_HEADERS } = require('opentracing')
    ...
    function handleOpentraceingHeaders(req, res, next) {
        req.tracer = {};
        const jaegerTracer = getJaegerTracer();
        if (jaegerTracer) {
            const parentSpanContext = tracer.extract(FORMAT_HTTP_HEADERS, req.headers);
            req.tracer.jaeger = { parentSpanContext };
        }
    }
    
  4. Add above middleware in express app if tracer is enabled. ParseServer.js

     static app({ maxUploadSize = '20mb', appId, directAccess, tracer }) {
        // add jaeger middleware if opentracing is enabled.
        if (tracer && tracer.jaeger && tracer.jaeger.enable) {
            api.use(middlewares.handleOpentraceingHeaders);
        }
     }
    
  5. Now we need to pass this req.tracer to triggers.js;

    function getRequestQueryObject(triggerType, auth, query, count, config, isGet, tracer) {
      isGet = !!isGet;
      var request = {
        triggerName: triggerType,
        query,
        master: false,
        count,
        log: config.loggerController,
        isGet,
        headers: config.headers,
        ip: config.ip,
        tracer
      };
        ...
    }
    
    function getRequestObject(triggerType, auth, parseObject, originalParseObject, config, context, tracer) {
      const request = {
        triggerName: triggerType,
        object: parseObject,
        master: false,
        log: config.loggerController,
        headers: config.headers,
        ip: config.ip,
        tracer
      };
      ...
    }
    
  6. Similar to Parse.Cloud.run create Parse.Tracer.getJaegerChildSpanHeaders()

    const { FORMAT_HTTP_HEADERS } = require('opentracing');
    
    function getJaegerChildSpan(spanName, spanOption) {
       const jaegerTracer = getJaegerTracer();
       if (!jaegerTracer) {
            return undefined;
       }
       return jaegerTracer.startSpan(spanName, spanOption);
    }
    
    function getJaegerSpanHeaders(span, headers) {
        headers = headers || {};
        const jaegerTracer = getJaegerTracer();
        if (jaegerTracer) {
            jaegerTracer.inject(span, FORMAT_HTTP_HEADERS, headers);
        }
        return headers;
    }
    
  7. Now pass the headers in options({ useMasterKey, sessionToken, headers })

  8. Pass this headers in new api calls from RESTController.js

    ...
    return RESTController.ajax(method, url, payloadString, options.headers || {}, options).then(
    ...
    

If the above proposed solution looks good then I can also create PR for the same.

@yog27ray
Copy link
Contributor Author

yog27ray commented Apr 7, 2020

Alternatively we can just provide some option to get request headers in all hooks and option to pass new headers while making Parse.Cloud.run, save(), find(), first(), etc. Jaeger Integration can be done in the user who need it.

@mtrezza
Copy link
Member

mtrezza commented Apr 22, 2020

The above mentioned case was just an example, we don't actually call a cloud-function from another cloud-function. But if cloud-function saves some object then beforeSave and afterSave trigger doesn't have information about source of origin.

You will be able to pass a context dictionary when you save an object: #6459. The context is accessible in beforeSave / afterSave. I can imagine this could be extended to cloud function calls in another PR, and gradually extended across all methods to a unified context. In such a context you could provide a request ID.

The feature is expected to be included in the next parse server release. If you do a quick PR for cloud function calls (or what it is that you specifically need) you could get your feature very soon.

@yog27ray
Copy link
Contributor Author

@mtrezza since your JS SDK PR is merged, I don't need to generate any other PR on JS SDK. Once your Server PR is merged. I will generate the PR for the other hooks.

@mtrezza
Copy link
Member

mtrezza commented Apr 26, 2020

What functionality do you want to achieve?

If you want to expose context to find, first as you mentioned that would require a PR in the JS SDK. The merged change in parse-community/Parse-SDK-JS#1150 only affects the save method.

Sent with GitHawk

@yog27ray
Copy link
Contributor Author

yog27ray commented May 1, 2020

@mtrezza, Yes I missed query function will create PR for the same also.

@mtrezza
Copy link
Member

mtrezza commented Jul 2, 2020

This is also (in some way) related to #6744 which introduces the new http header X-Parse-Request-Id. It is a feature in the works that allows any SDK to turn on sending a request ID in the headers to Parse Server. On top of that it would be possible to use that request ID and pass it on with each call to trace the function calls.

Worth mentioning that load balancers usually allow to append a trace ID http header to an incoming request, so that could also be used to trace internal calls by passing them on internally. For inspiration, you can take a look at AWS ELB or nginx how they augment the trace ID each time it passes through the load balancer.

@mtrezza mtrezza removed the discussion label Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

4 participants