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

Enable engine reporting in Lambda #1313

Merged
merged 9 commits into from
Jul 10, 2018
Merged

Conversation

evans
Copy link
Contributor

@evans evans commented Jul 9, 2018

This PR adds the ability to report from a stateless environment by adding the option to disable the interval that the engine reporting agent uses.

Ideally there would be a way to determine when the lambda is spinning down and gracefully send a report, however a quick skim of the documentation does not illicit any nice hook. @adnsio if you have any advice, I'd love to hear your thoughts.

@ghost ghost added the ⛲️ feature New addition or enhancement to existing solutions label Jul 9, 2018
@evans evans requested a review from martijnwalraven July 9, 2018 06:43
@evans evans added this to the Release 2.0 milestone Jul 9, 2018
@@ -316,3 +316,7 @@ addMockFunctionsToSchema({
itself. Set this to false to disable. You can manually invoke 'stop()' and
'sendReport()' on other signals if you'd like. Note that 'sendReport()'
does not run synchronously so it cannot work usefully in an 'exit' handler.

* `disableInterval`: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to expose this on ApolloServer? I would expect it to be a setting that particular integrations (like the Lambda one) can turn on.

@@ -79,6 +79,8 @@ export interface EngineReportingOptions {
// 'sendReport()' on other signals if you'd like. Note that 'sendReport()'
// does not run synchronously so it cannot work usefully in an 'exit' handler.
handleSignals?: boolean;
// Disables reporting on an interval for stateless environments
disableInterval?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems clearer to me to turn this around, so instead of disableInterval maybe something like sendReportsImmediately.

this.options.reportIntervalMs || 10 * 1000,
);
this.sendReportsImmediately = options.sendReportsImmediately;
if (this.sendReportsImmediately) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks backwards to me. Should it be if (!this.sendReportsImmediately)?

@evans evans merged commit e29f804 into version-2 Jul 10, 2018
@evans evans deleted the server-2.0/lambda-engine-reporting branch July 10, 2018 02:29
@khaledosman
Copy link

I don't see this in the documentation here https://www.apollographql.com/docs/apollo-server/api/apollo-server/ , am I missing something?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⛲️ feature New addition or enhancement to existing solutions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants