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

TypeError in tracer.ts when getActiveConfig() returns undefined #14

Closed
palp opened this issue Jul 5, 2023 · 8 comments
Closed

TypeError in tracer.ts when getActiveConfig() returns undefined #14

palp opened this issue Jul 5, 2023 · 8 comments

Comments

@palp
Copy link

palp commented Jul 5, 2023

I have not attempted a simple repro so I don't know the root cause of this, but this error occurs in my worker reliably:

Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'sampling')                                                                                                                               
                                                                                                                                                                                                                                  
  const sampler = getActiveConfig().sampling.headSampler                                                                                                                                                                          
                                    ^
      at startSpan (src/tracer.ts:47:36)
      at startActiveSpan (src/tracer.ts:88:20)
      at apply (../src/instrumentation/do-storage.ts:72:17)
      at proxyHandler.apply (../src/instrumentation/wrap.ts:27:18)
      at  (C:\Users\palp\AppData\Local\Temp\tmp-35572-Q5UGGZxJVrjs/src/billing.ts:81:39)

I was able to resolve it by modifying getActiveConfig to return a default value instead of undefined, but this may or may not be appropriate/desired behavior.
main...palp:otel-cf-workers:main

@evanderkoogh
Copy link
Owner

Hmm.. that is certainly interesting. It shouldn't be possible for getActiveConfig to return undefined. But I just realised that it is possible to do a blockConcurrencyWhile in the constructor. And it might be possible for there not to be a config set.

Do you have that scenario in this particular case?

@palp
Copy link
Author

palp commented Jul 5, 2023

Hmm.. that is certainly interesting. It shouldn't be possible for getActiveConfig to return undefined. But I just realised that it is possible to do a blockConcurrencyWhile in the constructor. And it might be possible for there not to be a config set.

Do you have that scenario in this particular case?

Yep, there is a call to storage inside a blockConcurrencyWhile call which is triggering it.

@palp
Copy link
Author

palp commented Jul 5, 2023

Yes, I have confirmed reproduction with this test class:

import { instrumentDO, ResolveConfigFn } from '@microlabs/otel-cf-workers'

export interface Env {
	OTEL_BASIC_AUTH: string;
	OTEL_URL: string;
}


class TestDO implements DurableObject {
	constructor(state: DurableObjectState, env: Env) {
		state.blockConcurrencyWhile(async () => {
			await state.storage.list({ prefix: 'test' })

		})
	}
	async fetch(request: Request) {
		return new Response('Hello world!', { status: 200 })
	}
}

const config: ResolveConfigFn = (env: Env, _trigger) => {
	return {
		exporter: { url: env.OTEL_URL, headers: { 'Authorization': `Basic ${env.OTEL_BASIC_AUTH}` } },
		service: { name: 'test'},
	}
}

const TestOtelDO = instrumentDO(TestDO, config)
export { TestOtelDO }

Note it only fails if the worker calling the DO is also instrumented, otherwise it seems to work as expected.

@evanderkoogh
Copy link
Owner

Awesome! Thanks for the repro.. I am trying to work out the best way to fix this. Hopefully I'll have it fixed in the next few hours :)

@evanderkoogh
Copy link
Owner

Ok.. a fix for this should be released as part of 1.0.0-rc3. The only annoying thing is that the constructor has to be in its own trace, because the fetch method hasn't been called and thus we have no idea what trace (or traces even) are waiting for the constructor to finish.

@SamyPesse
Copy link

SamyPesse commented Jul 7, 2023

I'm observing this issue when running tests locally with miniflare where at some point getActiveConfig starts returning undefined. I'll continue investigating, I've opened #20 as a workaround.

EDIT: it looks related to how miniflare re-initializes the DO in between requests.

@SamyPesse
Copy link

Also got the error in production in a DurableObject, not sure why but the pattern is globally:

  1. I have a durable Object DOClass
  2. This class creates a new object in its constructor this.something = new SomethingWithStorage(this.state.storage)
  3. Then when handling a WebSocket message, I call this.something.doSOmething() which uses the storage
  4. 💥

Using 1.0.0-rc.6

@jrowny
Copy link

jrowny commented Mar 22, 2024

Also getting this same error, same exact situation and repro as @SamyPesse

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

No branches or pull requests

4 participants