Skip to content

Commit

Permalink
Add instrumentationsLoaded as a helper
Browse files Browse the repository at this point in the history
Instead of calling `instrumentationsLoaded()` from a client property,
add it as an isolated helper function so users' apps keep working if
they haven't initialized the AppSignal client.
  • Loading branch information
luismiramirez committed Jan 18, 2023
1 parent a8b9781 commit 0a14594
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 6 deletions.
4 changes: 2 additions & 2 deletions .changesets/add-opentelemetry-sdk-initialization-to-client.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ bump: "patch"
type: "add"
---

Add OpenTelemetry SDK initialization to client
Add OpenTelemetry SDK initialization helper function

The OpenTelemetry instrumentations are loaded async, this is OK when users rely on automatic instrumentations, but for custom instrumentations
it might cause spans not to be reported. There's a new property in the client that contains the instrumentations registration that can be awaited for resolution before running any custom instrumentations.
it might cause spans not to be reported. There's a new helper function that contains the instrumentations registration that can be awaited for resolution before running any custom instrumentations.
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 23 additions & 1 deletion src/__tests__/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import {
setNamespace,
setRootName,
setError,
sendError
sendError,
instrumentationsLoaded
} from "../helpers"

function throwError() {
Expand Down Expand Up @@ -286,4 +287,25 @@ describe("Helpers", () => {
expect(debugMock).not.toHaveBeenCalled()
})
})

describe("instrumentationsLoaded", () => {
it("returns a promise from the globally stored client if found", () => {
const debugMock = jest.spyOn(Client.integrationLogger, "debug")

expect(instrumentationsLoaded()).toBeInstanceOf(Promise)
expect(debugMock).toHaveBeenCalledTimes(0)
})

it("returns an empty promise and logs if the globally stored client is not found", () => {
// Remove the stored client
global.__APPSIGNAL__ = null as any

const debugMock = jest.spyOn(Client.integrationLogger, "debug")

expect(instrumentationsLoaded()).toBeInstanceOf(Promise)
expect(debugMock).toHaveBeenCalledWith(
"Client is not initialized, cannot get OpenTelemetry instrumentations loaded"
)
})
})
})
3 changes: 2 additions & 1 deletion src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class Client {
config: Configuration
readonly integrationLogger: IntegrationLogger
extension: Extension
instrumentationsLoaded?: Promise<void>
instrumentationsLoaded: Promise<void>

#metrics: Metrics
#sdk?: NodeSDK
Expand Down Expand Up @@ -141,6 +141,7 @@ export class Client {
this.config = new Configuration(options)
this.extension = new Extension()
this.integrationLogger = this.setUpIntegrationLogger()
this.instrumentationsLoaded = Promise.resolve()
this.storeInGlobal()

if (this.isActive) {
Expand Down
13 changes: 13 additions & 0 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,16 @@ export function sendError(error: Error, fn: () => void = () => {}) {
)
}
}

export function instrumentationsLoaded(): Promise<void> {
const globallyStoredClient = Client.client

if (globallyStoredClient) {
return globallyStoredClient.instrumentationsLoaded
} else {
Client.integrationLogger.debug(
"Client is not initialized, cannot get OpenTelemetry instrumentations loaded"
)
return Promise.resolve()
}
}

0 comments on commit 0a14594

Please sign in to comment.