feat(otel): Add OpenTelemetry middleware #901
Conversation
| const PACKAGE_NAME = '@hono/otel' | ||
| const PACKAGE_VERSION = '0.1.0' |
There was a problem hiding this comment.
I wanted to eliminate the duplication with the metadata in package.json, but couldn't think of a good way to do it.
There was a problem hiding this comment.
It must get the version number from package.json since Changesets will change the number when the PR is merged automatically.
There was a problem hiding this comment.
Sorry, as I'm not familiar with packaging JavaScript packages, I have no idea how can I extract the version number from package.json file. Actually, I tried to import it like:
import metadata from '../package.json' with { type: 'json' }
const PACKAGE_NAME = metadata.name
const PACKAGE_VERSION = metadata.version… but it didn't work due to the following errors from tsc:
src/index.ts(12,22): error TS2732: Cannot find module '../package.json'. Consider using '--resolveJsonModule' to import module with '.json' extension.
src/index.ts(12,40): error TS2823: Import attributes are only supported when the '--module' option is set to 'esnext' or 'nodenext'.
Could you give me some idea for this?
There was a problem hiding this comment.
How about trying to use the following tsconfig.json?
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"rootDir": "./src",
"outDir": "./dist",
"module": "ESNext",
"resolveJsonModule": true
},
"include": [
"src/**/*.ts"
],
}
|
Does this also work with cloudflare workers? After install the node-sdk of opentelemetry I am getting startup errors. Looks like that package isn't supported with cloudflare workers |
I have no idea either, but @microlabs/otel-cf-workers instead of @opentelemetry/sdk-node might work with Cloudflare Workers? |
|
Ah okay. Yeah I made an issue there about Hono compatibility. Looks like Hono exposes things a bit different than the default Cloudflare worker endpoint stuff. |
| const tracer = tracerProvider.getTracer(PACKAGE_NAME, PACKAGE_VERSION) | ||
| const route = c.req.matchedRoutes[c.req.matchedRoutes.length - 1] | ||
| await tracer.startActiveSpan( | ||
| `${c.req.method} ${route.path}`, |
There was a problem hiding this comment.
Is c.req.path not good instead of route.path?
There was a problem hiding this comment.
Oh, yeah, it's intended! According to OpenTelemetry's Semantic Conventions for HTTP Spans:
HTTP span names SHOULD be
{method} {target}if there is a (low-cardinality)targetavailable. If there is no (low-cardinality){target}available, HTTP span names SHOULD be{method}.
The following guidelines for span names will help you understand what we mean by “low-cardinality”:
The span name concisely identifies the work represented by the Span, for example, an RPC method name, a function name, or the name of a subtask or stage within a larger computation. The span name SHOULD be the most general string that identifies a
(statistically) interesting class of Spans, rather than individual Span instances while still being human-readable. That is, "get_user" is a reasonable name, while "get_user/314159", where "314159" is a user ID, is not a good name due to its high cardinality. Generality SHOULD be prioritized over human-readability.For example, here are potential span names for an endpoint that gets a
hypothetical account information:
Span Name Guidance getToo general get_account/42Too specific get_accountGood, and account_id=42 would make a nice Span attribute get_account/{accountId}Also good (using the "HTTP route")
|
Hi @dahlia ! Sorry for the late reply. This is good. I've left the comments and one question. What platforms can this middleware run on? It does not work on Cloudflare Workers, but are there runtimes it does not work on? I think you have to write notes about it on README. |
|
This middleware addresses honojs/hono#1864. For usage, see also the README file.