Skip to content

feat(node): Add basic cloud resource context #8764

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 1 commit into from
Aug 9, 2023
Merged

Conversation

lforst
Copy link
Contributor

@lforst lforst commented Aug 9, 2023

Resolves #8747

Adds a bit of cloud resource context based on environment variables that may or may not be available in certain cloud environments.

Note that I didn't manually check all of these environment variables but based them off of stuff I've found on the internet. I tried to put links to the places where I found them but for some of them I lost the link. For these instances GitHub codesearch shows a few results though so I kinda trust them.

@lforst lforst requested review from Lms24 and mydea August 9, 2023 09:24
'cloud.provider': 'gcp',
};
} else if (process.env.ALIYUN_REGION_ID) {
// TODO: find where I found these environment variables - at least gc.github.com returns something
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that I haven't figured this out yet. This is not a leftover in this PR but an actual todo.

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

LGTM! RE origin of env variables, I think as long as the providers we're most interested in are covered we should be good. As discussed offline, let's add cloudflare but otherwise :shipit:

I'm not sure if other SDKs already send this context but if they do we might want to cross-check the env variables.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2023

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 74.96 KB (+0.14% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 31.11 KB (0%)
@sentry/browser - Webpack (gzipped) 21.81 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 69.58 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 28.15 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped) 20.17 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 219.29 KB (+0.11% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 84.67 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 59.83 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 31.02 KB (+0.01% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 65.16 KB (+0.16% 🔺)
@sentry/react - Webpack (gzipped) 21.84 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 92.78 KB (+0.15% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 50.64 KB (+0.06% 🔺)

@lforst
Copy link
Contributor Author

lforst commented Aug 9, 2023

let's add cloudflare but otherwise

I just tried to find some way of detecting a CF env but I failed. We could do it similar to python where we look at incoming request headers but that's a job for future Luca.

@lforst lforst enabled auto-merge (squash) August 9, 2023 09:59
@lforst lforst merged commit 6b3db7b into develop Aug 9, 2023
@lforst lforst deleted the lforst-cloud-context branch August 9, 2023 10:05
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.

Add cloud resource context
2 participants