Skip to content

Improve logging #29

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

Merged
merged 3 commits into from
Nov 21, 2017
Merged

Improve logging #29

merged 3 commits into from
Nov 21, 2017

Conversation

etpinard
Copy link
Contributor

resolves #2

head: info.head,
processingTime: info.processingTime
}
}))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which looks like:

image

does that look ok @scjody ?

Copy link
Member

Choose a reason for hiding this comment

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

perhaps we could use a severity field in the after-export section. maybe 'INFO'?

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks good to me. Have you been able to deploy it to stage and see if it looks OK on Stackdriver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. I just tried doing that via plotbot, but I got:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I've fixed Plotbot permissions, but there's no Docker image to deploy.

screen shot 2017-11-16 at 17 14 30

The Docker build needs to succeed before you can deploy.

Copy link
Contributor

Choose a reason for hiding this comment

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

(This may have been an intermittent CircleCI issue so I'm going to retry the build.)

Copy link
Contributor

Choose a reason for hiding this comment

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

The rebuild worked. Now I'm dealing with a different deployment issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, that worked. The latest version of this branch has been deployed to stage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if it looks OK on Stackdriver?

From a private convo with @scjody :

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

perhaps we could use a severity field in the after-export section. maybe 'INFO'?

done in cbb4e13

@etpinard
Copy link
Contributor Author

cc @bpostlethwaite who might be interested in helping out.

}
})

app.on('export-error', (info) => {
if (SHOW_LOGS) {
console.log(`export error ${info.code} - ${info.msg}`)
console.log(JSON.stringify({
severity: 'ERROR',
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

console.warn(err)
if (SHOW_LOGS) {
console.log(JSON.stringify({
severity: 'ERROR',
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@etpinard
Copy link
Contributor Author

@scjody @bpostlethwaite wanna give this another 👁️ ?

Copy link
Contributor

@scjody scjody left a comment

Choose a reason for hiding this comment

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

💃

@etpinard etpinard merged commit 75fcf45 into master Nov 21, 2017
@etpinard etpinard deleted the logging branch November 21, 2017 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

log output
3 participants