-
Notifications
You must be signed in to change notification settings - Fork 1k
refactor(resources): use runtime check for default service name #6257
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| package-lock=false | ||
| install-links=false |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| export default function RootLayout({ children }) { | ||
| return ( | ||
| <html lang="en"> | ||
| <body>{children}</body> | ||
| </html> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export default function Home() { | ||
| return <h1>OTel Edge Runtime Test</h1>; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { defaultServiceName } from '@opentelemetry/resources'; | ||
|
|
||
| export function middleware(request) { | ||
| const serviceName = defaultServiceName(); | ||
| console.log('Service name:', serviceName); | ||
| return Response.next(); | ||
| } | ||
|
|
||
| export const config = { | ||
| matcher: '/api/:path*', | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| /** @type {import('next').NextConfig} */ | ||
| const nextConfig = { | ||
| webpack: (config, { dev }) => { | ||
| // Treat warnings as errors | ||
| config.plugins.push({ | ||
| apply: compiler => { | ||
| compiler.hooks.done.tap('FailOnWarnings', stats => { | ||
| if (stats.compilation.warnings.length > 0) { | ||
| console.error(stats.compilation.warnings.join('\n')); | ||
| throw new Error('Webpack build has warnings'); | ||
| } | ||
| }); | ||
| }, | ||
| }); | ||
| return config; | ||
| }, | ||
| }; | ||
|
|
||
| export default nextConfig; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| { | ||
| "name": "nextjs-15-edge-bundle-test", | ||
| "type": "module", | ||
| "private": true, | ||
| "scripts": { | ||
| "build": "rm -rf .next && next build", | ||
| "test:bundle": "npm i && npm run build" | ||
| }, | ||
| "dependencies": { | ||
| "@opentelemetry/resources": "file:../../../packages/opentelemetry-resources", | ||
| "next": "^15", | ||
| "react": "^18", | ||
| "react-dom": "^18" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "name": "nextjs-15-edge-bundle-test" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| package-lock=false | ||
| install-links=false |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { defaultServiceName } from '@opentelemetry/resources'; | ||
|
|
||
| export const runtime = 'edge'; | ||
|
|
||
| export function GET(request) { | ||
| const serviceName = defaultServiceName(); | ||
| return new Response(JSON.stringify({ serviceName }), { | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| export default function RootLayout({ children }) { | ||
| return ( | ||
| <html lang="en"> | ||
| <body>{children}</body> | ||
| </html> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export default function Home() { | ||
| return <h1>OTel Edge Runtime Test</h1>; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| /** @type {import('next').NextConfig} */ | ||
| const nextConfig = {}; | ||
|
|
||
| export default nextConfig; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| /** | ||
| * Next.js 16+ uses Turbopack, which lacks callbacks for build errors or warnings. | ||
| * This script runs the build and fails if Turbopack reports any warnings. | ||
| */ | ||
| import { execSync } from 'node:child_process'; | ||
| import path from 'node:path'; | ||
|
|
||
| const testDir = path.dirname(new URL(import.meta.url).pathname); | ||
|
|
||
| // Install dependencies and run build, capturing both stdout and stderr | ||
| const output = execSync('npm run build 2>&1', { | ||
| cwd: testDir, | ||
| encoding: 'utf-8', | ||
| }); | ||
|
|
||
| const hasWarning = /Turbopack build encountered/.test(output); | ||
| if (hasWarning) { | ||
| console.error('Build produced warnings:\n', output); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| console.log('Build completed without warnings'); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| { | ||
| "name": "nextjs-16-edge-bundle-test", | ||
| "type": "module", | ||
| "private": true, | ||
| "scripts": { | ||
| "build": "rm -rf .next && next build", | ||
| "test:bundle": "npm i && npx tsx next.test.ts" | ||
| }, | ||
| "dependencies": { | ||
| "@opentelemetry/resources": "file:../../../packages/opentelemetry-resources", | ||
| "next": "^16", | ||
| "react": "^19", | ||
| "react-dom": "^19" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "name": "nextjs-16-edge-bundle-test" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,15 +14,26 @@ | |
| * limitations under the License. | ||
| */ | ||
|
|
||
| // Check if we are in a Node.js environment and if so, use the process.argv0 property | ||
| // to determine the default service name | ||
| const DEFAULT_SERVICE_NAME = | ||
| typeof process === 'object' && | ||
| typeof process.argv0 === 'string' && | ||
| process.argv0.length > 0 | ||
| ? `unknown_service:${process.argv0}` | ||
| : 'unknown_service'; | ||
| let serviceName: string | undefined; | ||
|
|
||
| /** | ||
| * Returns the default service name for OpenTelemetry resources. | ||
| * In Node.js environments, returns "unknown_service:<process.argv0>". | ||
| * In browser/edge environments, returns "unknown_service". | ||
| */ | ||
| export function defaultServiceName(): string { | ||
| return DEFAULT_SERVICE_NAME; | ||
| if (serviceName === undefined) { | ||
| try { | ||
| const argv0 = globalThis.process.argv0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Oops, hasn't been released yet. My bad 😅.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No worries. Give it a shot now @andreiborza There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It worked: getsentry/sentry-javascript#18934 Thanks for taking care of this!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw your comment on another PR about unclear Edge Runtime support. The Browser SIG has not discussed it but I'll bring it up on Slack to see if the JS SIG has. Feel free to join us there. |
||
| serviceName = argv0 ? `unknown_service:${argv0}` : 'unknown_service'; | ||
| } catch { | ||
| serviceName = 'unknown_service'; | ||
| } | ||
| } | ||
| return serviceName; | ||
| } | ||
|
|
||
| /** @internal For testing purposes only */ | ||
| export function _clearDefaultServiceNameCache(): void { | ||
| serviceName = undefined; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked at #6208 (comment) whether potentially using
global.process?.argv0would suffice to avoid whatever static analysis warnings.It would also be nice to have specific examples of these static analysis warnings to help for motivation in future maintenance.