Skip to content

fix(metrics-opentelemetry): isXXX function aren't correct #2918

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.

Sign up for GitHub

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

Closed
sfroment wants to merge 2 commits into from
Closed

fix(metrics-opentelemetry): isXXX function aren't correct #2918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a2928f
fix(metrics-opentelemetry): isXXX function aren't correct
sfroment Jan 20, 2025
39e84ab
Merge branch 'main' into main
achingbrain Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions packages/metrics-opentelemetry/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ export function openTelemetryMetrics (init: OpenTelemetryMetricsInit = {}): (com
return (components: OpenTelemetryComponents) => new OpenTelemetryMetrics(components, init)
}

function isPromise <T = any> (obj?: any): obj is Promise<T> {
export function isPromise <T = any> (obj?: any): obj is Promise<T> {
return typeof obj?.then === 'function'
}

Expand All @@ -458,8 +458,13 @@ async function wrapPromise (promise: Promise<any>, span: Span, attributes: Trace
})
}

function isGenerator (obj?: any): obj is Generator {
return obj?.[Symbol.iterator] != null
export function isGenerator (obj: unknown): obj is Generator {
if (obj == null) return false
const iterator = (obj as { [Symbol.iterator]?: unknown })?.[Symbol.iterator]
if (typeof iterator !== 'function') return false

const instance = obj as { next?: unknown }
return typeof instance.next === 'function'
}

function wrapGenerator (gen: Generator, span: Span, attributes: TraceAttributes, options?: TraceGeneratorFunctionOptions<any, any, any>): Generator {
Expand Down Expand Up @@ -502,8 +507,15 @@ function wrapGenerator (gen: Generator, span: Span, attributes: TraceAttributes,
return wrapped
}

function isAsyncGenerator (obj?: any): obj is AsyncGenerator {
return obj?.[Symbol.asyncIterator] != null
export function isAsyncGenerator (obj: unknown): obj is AsyncGenerator {
if (obj == null) return false
const asyncIterator = (obj as { [Symbol.asyncIterator]?: unknown })?.[
Symbol.asyncIterator
]
if (typeof asyncIterator !== 'function') return false

const instance = obj as { next?: unknown }
return typeof instance.next === 'function'
}

function wrapAsyncGenerator (gen: AsyncGenerator, span: Span, attributes: TraceAttributes, options?: TraceGeneratorFunctionOptions<any, any, any>): AsyncGenerator {
Expand Down
111 changes: 110 additions & 1 deletion packages/metrics-opentelemetry/test/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from 'aegir/chai'
import { openTelemetryMetrics } from '../src/index.js'
import { isPromise, isGenerator, isAsyncGenerator, openTelemetryMetrics } from '../src/index.js'

describe('opentelemetry-metrics', () => {
it('should wrap a method', async () => {
Expand All @@ -23,3 +23,112 @@
expect(wrapped).to.not.equal(target.wrapped)
})
})

describe('isPromise', () => {
it('should return true if the value is a promise', () => {
expect(isPromise(Promise.resolve())).to.be.true()
expect(isPromise(new Promise(() => {}))).to.be.true()
expect(isPromise(Promise.reject(new Error('test')))).to.be.true()
})

it('should return false if the value is not a promise', () => {
expect(isPromise(1)).to.be.false()
expect(isPromise('string')).to.be.false()
expect(isPromise({})).to.be.false()
expect(isPromise([])).to.be.false()
expect(isPromise(null)).to.be.false()
expect(isPromise(undefined)).to.be.false()
expect(isPromise(() => {})).to.be.false()
expect(isPromise(async () => {})).to.be.false()
expect(
isPromise(function * () {
yield 1

Check warning on line 45 in packages/metrics-opentelemetry/test/index.spec.ts

View check run for this annotation

Codecov / codecov/patch

packages/metrics-opentelemetry/test/index.spec.ts#L45 

Added line #L45 was not covered by tests
})
).to.be.false()
expect(
isPromise(async function * () {
yield 1

Check warning on line 50 in packages/metrics-opentelemetry/test/index.spec.ts

View check run for this annotation

Codecov / codecov/patch

packages/metrics-opentelemetry/test/index.spec.ts#L50 

Added line #L50 was not covered by tests
})
).to.be.false()
// biome-ignore lint/suspicious/noThenProperty: for testing purposes
expect(isPromise({ then: 1 })).to.be.false()
})
})

describe('isGenerator', () => {
it('should return true if the value is a generator', () => {
function * gen (): Generator<number> {
yield 1
}

Check warning on line 62 in packages/metrics-opentelemetry/test/index.spec.ts

View check run for this annotation

Codecov / codecov/patch

packages/metrics-opentelemetry/test/index.spec.ts#L62 

Added line #L62 was not covered by tests
const generator = gen()
expect(isGenerator(generator)).to.be.true()

const genObj = (function * () {
yield 1
})()
expect(isGenerator(genObj)).to.be.true()
})

it('should return false if the value is not a generator', () => {
expect(isGenerator(1)).to.be.false()
expect(isGenerator('string')).to.be.false()
expect(isGenerator({})).to.be.false()
expect(isGenerator([])).to.be.false()
expect(isGenerator(null)).to.be.false()
expect(isGenerator(undefined)).to.be.false()
expect(isGenerator(() => {})).to.be.false()
expect(isGenerator(async () => {})).to.be.false()
expect(
isGenerator(function * () {
yield 1

Check warning on line 83 in packages/metrics-opentelemetry/test/index.spec.ts

View check run for this annotation

Codecov / codecov/patch

packages/metrics-opentelemetry/test/index.spec.ts#L83 

Added line #L83 was not covered by tests
})
).to.be.false() // generator function, not generator
expect(
isGenerator(async function * () {
yield 1

Check warning on line 88 in packages/metrics-opentelemetry/test/index.spec.ts

View check run for this annotation

Codecov / codecov/patch

packages/metrics-opentelemetry/test/index.spec.ts#L88 

Added line #L88 was not covered by tests
})
).to.be.false()
expect(isGenerator(Promise.resolve())).to.be.false()
expect(isGenerator({ next: () => {} })).to.be.false()
expect(isGenerator({ [Symbol.iterator]: () => {} })).to.be.false()
})
})

describe('isAsyncGenerator', () => {
it('should return true if the value is an async generator', () => {
async function * asyncGen (): AsyncGenerator<number> {
yield 1
}

Check warning on line 101 in packages/metrics-opentelemetry/test/index.spec.ts

View check run for this annotation

Codecov / codecov/patch

packages/metrics-opentelemetry/test/index.spec.ts#L101 

Added line #L101 was not covered by tests
const asyncGenerator = asyncGen()
expect(isAsyncGenerator(asyncGenerator)).to.be.true()

const asyncGenObj = (async function * () {
yield 1
})()
expect(isAsyncGenerator(asyncGenObj)).to.be.true()
})

it('should return false if the value is not an async generator', () => {
expect(isAsyncGenerator(1)).to.be.false()
expect(isAsyncGenerator('string')).to.be.false()
expect(isAsyncGenerator({})).to.be.false()
expect(isAsyncGenerator([])).to.be.false()
expect(isAsyncGenerator(null)).to.be.false()
expect(isAsyncGenerator(undefined)).to.be.false()
expect(isAsyncGenerator(() => {})).to.be.false()
expect(isAsyncGenerator(async () => {})).to.be.false()
expect(
isAsyncGenerator(function * () {
yield 1

Check warning on line 122 in packages/metrics-opentelemetry/test/index.spec.ts

View check run for this annotation

Codecov / codecov/patch

packages/metrics-opentelemetry/test/index.spec.ts#L122 

Added line #L122 was not covered by tests
})
).to.be.false()
expect(
isAsyncGenerator(async function * () {
yield 1

Check warning on line 127 in packages/metrics-opentelemetry/test/index.spec.ts

View check run for this annotation

Codecov / codecov/patch

packages/metrics-opentelemetry/test/index.spec.ts#L127 

Added line #L127 was not covered by tests
})
).to.be.false() // async generator function, not generator
expect(isAsyncGenerator(Promise.resolve())).to.be.false()
expect(isAsyncGenerator({ next: async () => {} })).to.be.false()
expect(isAsyncGenerator({ [Symbol.asyncIterator]: () => {} })).to.be.false()
})
})
Loading