Add destination mw

[aultimus]

https://segment.atlassian.net/browse/LIBWEB-168

This pull request adds destination middleware to AJS and deprecates integration middleware. Destination middleware is similar to integration middleware, the key difference being in the interface for how it is added to AJS:

Destination Middleware:
addDestinationMiddleware(integrationName, middlewares)

Integration Middleware:
addIntegrationMiddleware(middleware)

Destination middleware only applies to the specific integration specified in the call to addDestinationMiddleware whereas integration middleware applies to all enabled integrations. The middlewares argument is an array which can contain one or many middlewares to add for this destination.

This is an example of adding a destination middleware that sets the username to 'foo' for the Segment.io destination:

f1=function(payload, integration,next) {payload.obj.userId = "foo"; next(payload);};
analytics.addDestinationMiddleware("Segment.io", [f1]);

Validation

run this code in the dev console of a source with this version of ajs core deployed, observe that the associated source will display userId of 'foobarcat' in the segment debugger.

f=function({payload, next, integrations}) {payload.obj.userId = "foo"; next(payload);}; analytics.addSourceMiddleware(f);

f=function(payload, integration, next) {payload.obj.userId = payload.obj.userId + "bar";next(payload);}; analytics.addIntegrationMiddleware(f);

f=function({payload, integration, next}) { payload.obj.userId = payload.obj.userId + "cat";next(payload);}; analytics.addDestinationMiddleware("Segment.io",[f]);

analytics.identify();

Testing completed successfully on staging. ✅ https://www.dropbox.com/s/ptsmf68vtnuwxp2/stage_add_destination_mw.mov?dl=0

Checklist

Please ensure the following are completed to help get your PR merged:

• Thorough explanation of the issue/solution, and a link to the related issue
• CI tests are passing
• Unit tests were written for any new code
• Code coverage is at least maintained, or increased.

Respect earns Respect 👏

Please respect our Code of Conduct, in short:

• Using welcoming and inclusive language.
• Being respectful of differing viewpoints and experiences.
• Gracefully accepting constructive criticism.
• Focusing on what is best for the community.
• Showing empathy towards other community members.

[juliofarah]

😍

[pooyaj]

Approach looks great! One minor comment.

[pooyaj]

In modern JS you can do mw({ payload, integration, next });. Unfortunately AJS is retro JS, so this needs to be

  mw({
          payload: payload,
          integration: integration,
          next: next,
      });

[fathyb]

we could move this block on the method body instead:

var self = this;
var chain = self._destinationMiddlewares[integrationName]
if (!chain) {
  chain = self._destinationMiddlewares[
    integrationName
  ] = new DestinationMiddlewareChain();
}
middlewares.forEach(function(middleware) {
  chain.add(middleware);

[fathyb]

we should keeps things DRY and create an internal method for this:

// ...
        this._invokeIntegration(method, integration, result)
    })
} else {
    this._invokeIntegration(method, integration, result)
// ...

Analytics.prototype._invokeIntegration = function (method, integration, payload) {
    self.emit("invoke", payload)
    metrics.increment("analytics_js.invoke", {
        method: method,
    })

    metrics.increment("analytics_js.integration.invoke", {
        method: method,
        integration_name: integration.name,
    })

    integration.invoke.call(integration, method, payload)
}

[pooyaj]

@aultimus thinking more about this, we need to put this back outside in the place that it was before. Right now we are doing self.emit('invoke', result); multiple times. Let's say the customer has enabled 10 integrations, we will emit the invoke 10 times ( and subsequently segment's own integration will record 10 events ). Right now the invoke event seems to be integration agnostic (unlike the integration.invoke.call(integration, method, result); call), therefore it should be triggered regardless of the destination middleware.

[pooyaj]

@aultimus did some testing on the the invoke emitter. Definitely the change is causing a regression. I have a source with 3 destinations, and I'm getting three invoke events when doing a track call. Check this out: https://cl.ly/257d0b64cfad
(using master branch I only get one event emitted)