feat(apollo-server-express): add support for apollo-server-express

[danieljuhl]

Added instrumentation for apollo-server-express. As the actual instrumentation happens in apollo-server-core it might also work with all other apollo-server-*, but hasn't been tested yet.

Fixes #641

Checklist

• Implement code
• Add tests

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[danieljuhl]

@Qard @watson can any of you guide me to fixing/handling the failing test with node v4. apollo-server-express requires node >=6.

[watson]

@danieljuhl looks like you found the answer your self by adding the if (semver.lt(process.version, '6.0.0')) process.exit() line and moving the require statement below that. Now there's just a few double quotes that's making our linter angry. When those are fixed I think the tests should pass fine 👍

[danieljuhl]

@watson yes, I think I found the solution, and waiting for the tests to finally pass :)

[danieljuhl]

@watson it looks to me, as if #646 is also causing this PR to fail. Can you confirm? And what do you want me to do from here.

[watson]

Sorry about that. I just merged #646 which should fix the issue you experienced. If you rebase with master, it should be all green now 😄

[watson]

I actually haven't reviewed the PR yet btw, so we'll just do that as well. So if you want you can just wait rebasing until afterwards

[Qard]

Just one suggestion on how to improve the code. Other than that though, it looks good to me. 👍

[watson]

Could I get you to add a row to this table in the documentation as well?

https://github.com/elastic/apm-agent-nodejs/blob/master/docs/compatibility.asciidoc#custom-transactions

[danieljuhl]

Thanks @watson and @Qard, I’ll update the PR tomorrow morning (rebase and rewrite per the review)

[danieljuhl]

@watson @Qard it still looks like there is some conflicts with the dependency tests. Can you guide me, if there is something I can do.

[watson]

I've just tried to fix it. Let's see what the tests say...

[danieljuhl]

I've just tried to fix it. Let's see what the tests say...

Still not a happy CI. Maybe we need to remove apollo-server-express before express-graphql?

[watson]

I wish npm would remove the concept of peer dependencies. It's nothing but trouble 😞

I'm trying now to split the tests for apollo-server-express and express-graphql into two separate builds. That way they should not interfere with each other.

[watson]

Didn't fix it... I'll have another look later

[watson]

Finally got the tests to pass 😅

@Qard have your review comments been addressed?

[danieljuhl]

Finally got the tests to pass 😅

Great! And thanks for the help..

@Qard have your review comments been addressed?

I've tried at least 😄and followed the review comments exactly, so I hope they have been addressed.

[Qard]

One last thing, and then it looks good to me. 👍