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

Research and fix issues with Next.js 14 #1014

Closed
5 tasks done
unflxw opened this issue Apr 8, 2024 · 17 comments
Closed
5 tasks done

Research and fix issues with Next.js 14 #1014

unflxw opened this issue Apr 8, 2024 · 17 comments
Assignees
Labels

Comments

@unflxw
Copy link
Contributor

unflxw commented Apr 8, 2024

Intercom: https://app.intercom.com/a/inbox/yzor8gyw/inbox/admin/5246522/conversation/16410700308049?view=List

Customer is using Next.js 14.1.3. Performance samples are reported as UNKNOWN /some-path -- indicating that http.method is not provided in the span that is treated as the root span.

We should research what's going on and fix it.

To do

(this is updated with issues from the comments below)

  • Fix UNKNOWN HTTP method
    • done, caused following issue:
    • Fix render/getServerSideProps HTTP method
      • done, probably caused following issue:
      • Fix issue with high cardinality action names
  • Research calls to Next.js fetch not being instrumented
    • working as expected (Next.js does not instrument fetch when using the Pages router), updated docs
  • Research (and fix, if possible) Server Components error reporting
@unflxw unflxw added the bug label Apr 8, 2024
@unflxw
Copy link
Contributor Author

unflxw commented Apr 8, 2024

The customer reports that calls to Next.js fetch are no longer instrumented. Research if Next.js 14 no longer emits OpenTelemetry spans for its custom fetch implementation?

@unflxw unflxw changed the title Research and fix missing HTTP method in Next.js 14 Research and fix issues with Next.js 14 Apr 8, 2024
@unflxw
Copy link
Contributor Author

unflxw commented Apr 8, 2024

The customer also ran into an issue when importing AppSignal's helpers from Next.js code, where Webpack fails to bundle AppSignal:

⨯ ./node_modules/@appsignal/nodejs/build/Release/extension.node
Module parse failed: Unexpected character '�' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)

See #114. The solution might be to ask customers to add us to the serverComponentsExternalPackages configuration option, or to add ourselves to the default bundled exclusion list, like other APM providers do.

@unflxw
Copy link
Contributor Author

unflxw commented Apr 11, 2024

Another customer (Intercom) reported an "AppSignal not starting, no valid configuration found" error, which is not of the same nature as the above issues, but should also be looked into.

@unflxw unflxw self-assigned this Apr 15, 2024
@unflxw
Copy link
Contributor Author

unflxw commented Apr 15, 2024

Finally got around to start testing this. Ran into the Webpack bundling issue head-on, and I was able to fix it using the serverComponentsExternalPackages configuration option. Will reach out to the customer.

unflxw added a commit to unflxw/next.js-fork that referenced this issue Apr 15, 2024
The `@appsignal/nodejs` instrumentation package fails to load in Next.js 14 due to Webpack failing to bundle its Node.js native extension. Adding it to the server components external packages list fixes this issue.

Fixes appsignal/appsignal-nodejs#1014.
@unflxw
Copy link
Contributor Author

unflxw commented Apr 15, 2024

Sent a PR to Vercel to add "@appsignal/nodejs" to the serverComponentsExternalPackages default list: vercel/next.js#64503

Reproduced the issue with the UNKNOWN method and implemented a fix for it in the agent: https://github.com/appsignal/appsignal-agent/pull/1129

@unflxw
Copy link
Contributor Author

unflxw commented Apr 15, 2024

I was not able to reproduce server-side calls performed with fetch not showing up. I will reach out to the customer.

@unflxw
Copy link
Contributor Author

unflxw commented Apr 17, 2024

Released the fix in https://github.com/appsignal/appsignal-agent/pull/1129 as AppSignal for Node.js 3.3.3 -- this, however, resulted in incorrect behaviour when using the Pages Router. I have done a "soft rollback" by resetting the latest tag to 3.3.2, and I am looking into a full fix.

The serverComponentsExternalPackages workaround also does not work when using the Pages Router (which, on hindsight, makes sense, given the name of the configuration option) and customers experience issues when manually importing @appsignal/nodejs in their Next.js pages to use our helpers.

The fetch server-side calls not showing up is, for whatever reason, expected behaviour when using the Pages Router. I have amended the documentation to clarify this.

unflxw added a commit to unflxw/next.js-fork that referenced this issue Apr 18, 2024
The `@appsignal/nodejs` instrumentation package fails to load in Next.js 14 due to Webpack failing to bundle its Node.js native extension. Adding it to the server components external packages list fixes this issue.

Fixes appsignal/appsignal-nodejs#1014.
@unflxw
Copy link
Contributor Author

unflxw commented Apr 18, 2024

I am not able to reproduce the issues that the customer using the Pages Router experienced. I have undone the "soft rollback" and latest is now 3.3.3.

A third customer (Intercom) has reported an issue where, when running Next.js (App Router) in production, errors emitted while rendering server components are intentionally obscured by Next.js itself.

ijjk pushed a commit to vercel/next.js that referenced this issue Apr 18, 2024
The `@appsignal/nodejs` instrumentation package fails to load in Next.js
14 due to Webpack failing to bundle its Node.js native extension. Adding
it to the server components external packages list fixes this issue.

Part of appsignal/appsignal-nodejs#1014.
@unflxw
Copy link
Contributor Author

unflxw commented Apr 23, 2024

I was able to reproduce the issue with render or getServerSideProps appearing instead of the method, which was reported by the customer. AppSignal for Node.js 3.3.4 was released with a fix to the issue.

@unflxw
Copy link
Contributor Author

unflxw commented Apr 23, 2024

A customer (Intercom link) has reported high cardinality action names after upgrading to 3.3.4. It seems that traces that should be ignored (because they do not have a proper root name) are not being properly ignored.

@backlog-helper
Copy link

backlog-helper bot commented May 8, 2024

  • This issue has not had any activity in 14 days. Please provide a status update if it is still relevant. Closed it if it is no longer relevant. Or move it to another column if it's blocked or requires another look at it. - (More info)

This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

@fabiolnm
Copy link

fabiolnm commented May 14, 2024

Struggling to fix this error when trying to instrument a Next.js 14 application, app router based.

Versions

"@appsignal/nodejs": "3.4.2"
"next": "14.2.3",

Error

 ○ Compiling /instrumentation ...
 ⨯ ./node_modules/@appsignal/nodejs/build/Release/extension.node
Module parse failed: Unexpected character '�' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)

Import trace for requested module:
./node_modules/@appsignal/nodejs/build/Release/extension.node
./node_modules/@appsignal/nodejs/dist/extension_wrapper.js
./node_modules/@appsignal/nodejs/dist/extension.js
./node_modules/@appsignal/nodejs/dist/client.js
./node_modules/@appsignal/nodejs/dist/index.js
./appsignal.cjs

next.config.js

module.exports = {
  reactStrictMode: true,
  experimental: {
    instrumentationHook: true,
    serverComponentsExternalPackages: [
      'docusign-esign',
      // https://docs.appsignal.com/nodejs/3.x/integrations/nextjs.html
      '@appsignal/nodejs'
    ],
  },
  webpack(config) {
    config.module.rules.push({
      test: /\.svg$/,
      use: ['@svgr/webpack'],
    })
    return config
  },
  async headers() {
    ...
  },
}

@fabiolnm
Copy link

It seems to be related to #928

@unflxw
Copy link
Contributor Author

unflxw commented May 14, 2024

Hi @fabiolnm,

Thank you for reporting this issue. Setting serverComponentsExternalPackages should fix this, but unfortunately I believe that serverComponentsExternalPackages is not honored when a custom webpack(config) is provided.

Could you check whether removing the custom webpack(config) fixes the issue? I know this is not a feasible fix, but it helps us understand what the problem is.

Could you also check whether adding node-loader to your Webpack configuration, as described in #928, fixes it?

@fabiolnm
Copy link

Same error with:

module.exports = {
  reactStrictMode: true,
  experimental: {
    instrumentationHook: true,
    serverComponentsExternalPackages: ['@appsignal/nodejs'],
  },
}

trace

 ○ Compiling /instrumentation ...
 ⨯ ./node_modules/@appsignal/nodejs/build/Release/extension.node
Module parse failed: Unexpected character '�' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)

Import trace for requested module:
./node_modules/@appsignal/nodejs/build/Release/extension.node
./node_modules/@appsignal/nodejs/dist/extension_wrapper.js
./node_modules/@appsignal/nodejs/dist/extension.js
./node_modules/@appsignal/nodejs/dist/client.js
./node_modules/@appsignal/nodejs/dist/index.js
./appsignal.cjs

@fabiolnm
Copy link

TLDR

Loader changes the nature of the error to other never-ending Can't resolve 'package_name' errors.

next.config.js

module.exports = {
  reactStrictMode: true,
  experimental: {
    instrumentationHook: true,
    serverComponentsExternalPackages: ['@appsignal/nodejs'],
  },
  webpack(config) {
    config.module.rules.push({
      test: /\.svg$/,
      use: ['@svgr/webpack'],
    })
    config.module.rules.push({
      test: /\.node$/i,
      loader: 'node-loader',
    })
    return config
  },
}

Can't resolve 'package_name' errors

 ○ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/call.js:21:1
Module not found: Can't resolve 'stream'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/client.js
./node_modules/@grpc/grpc-js/build/src/index.js
./node_modules/@opentelemetry/exporter-trace-otlp-grpc/build/src/OTLPTraceExporter.js
./node_modules/@opentelemetry/exporter-trace-otlp-grpc/build/src/index.js
./node_modules/@opentelemetry/sdk-node/build/src/TracerProviderWithEnvExporter.js
./node_modules/@opentelemetry/sdk-node/build/src/sdk.js
./node_modules/@opentelemetry/sdk-node/build/src/index.js
./node_modules/@appsignal/nodejs/dist/client.js
./node_modules/@appsignal/nodejs/dist/index.js
./appsignal.cjs
./src/instrumentation.ts

Trying to fix by installing packages

npm install stream && npm run dev
 ○ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/channel-credentials.js:20:1
Module not found: Can't resolve 'tls'
https://nextjs.org/docs/messages/module-not-found
Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/index.js


npm install tls && npm run dev
 ○ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/channelz.js:20:1
Module not found: Can't resolve 'net'
https://nextjs.org/docs/messages/module-not-found
Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/index.js

npm install net && npm run dev
○ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/compression-filter.js:20:1
Module not found: Can't resolve 'zlib'
https://nextjs.org/docs/messages/module-not-found
Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/internal-channel.js

Zlib

npm install browserify-zlib 

Also have to add a resolver alias to webpack config

config.resolve.alias.zlib = 'browserify-zlib'

Errors never end

npm run dev

○ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/http_proxy.js:23:1
Module not found: Can't resolve 'http'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/internal-channel.js

@unflxw
Copy link
Contributor Author

unflxw commented May 17, 2024

On @fabiolnm's issue above, in case someone else runs into it: make sure that, in instrumentation.ts, the call to require("./appsignal.cjs") is wrapped in if (process.env.NEXT_RUNTIME === "nodejs") -- this is important, as it tells Next not to attempt to bundle it for the browser.

Everything else is being looked at, so I am closing this issue for now. If you're encountering any issues with Next.js 14, please reach out to us at [email protected], or open a new issue.

@unflxw unflxw closed this as completed May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants