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

Add telemetry to HTTP requests #23

Merged
merged 1 commit into from
Sep 30, 2019
Merged

Add telemetry to HTTP requests #23

merged 1 commit into from
Sep 30, 2019

Conversation

joshcanhelp
Copy link
Contributor

Description

Add a telemetry header to HTTP requests.

Testing

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • All active GitHub checks for tests, formatting, and security are passing

@joshcanhelp joshcanhelp requested a review from a team September 27, 2019 21:16
@@ -4,9 +4,18 @@ const url = require('url');
const urlJoin = require('url-join');
const pkg = require('../package.json');

const telemetryHeader = {
name: 'express-oidc',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shortened the name a bit but still recognizable.

custom.setHttpOptionsDefaults({
headers: {
'User-Agent': `${pkg.name}/${pkg.version} (${pkg.homepage})`
'User-Agent': `${pkg.name}/${pkg.version}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this a good user agent? I removed the homepage, seems like unnecessary data.

Choose a reason for hiding this comment

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

@joshcanhelp Do you care about Node version or possibly the OS?

If not, package name and version is probably enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Node version is sent in the env object. We don't parse the user agent (at this point) for telemetry.

before(async function() {
client = await getClient(config);

nock('https://flosser.auth0.com')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nock was the best way I could think of/find for getting the options being used. They are not (understandably) exposed outside of the library.

@joshcanhelp joshcanhelp merged commit b2b2c72 into auth0:master Sep 30, 2019
@joshcanhelp joshcanhelp deleted the add-telemetry branch September 30, 2019 14:56
@joshcanhelp joshcanhelp added this to the v0.5.0 milestone Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants