Skip to content

Commit

Permalink
Allow nested values in setSampleData
Browse files Browse the repository at this point in the history
Fix the signature of setSampleData so that it accepts any array or
object, instead of only arrays or objects with up to two levels of
nesting. This allows the user to pass arbitrary objects to sample
data properties that accept them, such as `custom_data`.

Note that not all sample data properties accept nested values.
  • Loading branch information
unflxw committed Mar 9, 2022
1 parent d159543 commit 4a498be
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 31 deletions.
5 changes: 0 additions & 5 deletions packages/express/src/middleware/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ export function expressMiddleware(appsignal: Client): RequestHandler {
span.setName(`${method} ${baseUrl}${req.route.path}`)
}

// defeated the type checker here because i'm pretty sure the error
// `tsc` returns is actually a parse error
// @TODO: keep an eye on this
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
span.setSampleData("params", { ...params, ...query })
span.setSampleData("environment", filteredHeaders)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
bump: "patch"
type: "fix"
---

Allow nested values in `Span.setSampleData`. This change also allows
values other than strings, integers and booleans to be passed as values
within the sample data objects. Note that not all sample data keys allow
nested values to be passed.
13 changes: 2 additions & 11 deletions packages/nodejs/src/interfaces/span.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { HashMap, HashMapValue } from "@appsignal/types"
import { HashMap } from "@appsignal/types"

/**
* The state of a `Span` at initialization time.
Expand Down Expand Up @@ -87,16 +87,7 @@ export interface Span {
/**
* Sets a data collection as sample data on the current `Span`.
*/
setSampleData(
key: string,
data:
| Array<
HashMapValue | Array<HashMapValue> | HashMap<HashMapValue> | undefined
>
| HashMap<
HashMapValue | Array<HashMapValue> | HashMap<HashMapValue> | undefined
>
): this
setSampleData(key: string, data: Array<any> | HashMap<any>): this

/**
* Adds sanitized SQL data as a string to a Span.
Expand Down
5 changes: 1 addition & 4 deletions packages/nodejs/src/noops/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,7 @@ export class NoopSpan implements Span {
return this
}

public setSampleData(
_key: string,
_data: Array<string | number | boolean> | HashMap<string | number | boolean>
): this {
public setSampleData(_key: string, _data: Array<any> | HashMap<any>): this {
return this
}

Expand Down
13 changes: 2 additions & 11 deletions packages/nodejs/src/span.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { HashMap, HashMapValue } from "@appsignal/types"
import { HashMap } from "@appsignal/types"
import { Span, SpanOptions, SpanContext, SpanData } from "./interfaces"

import { span } from "./extension_wrapper"
Expand Down Expand Up @@ -114,16 +114,7 @@ export class BaseSpan implements Span {
/**
* Sets a data collection as sample data on the current `Span`.
*/
public setSampleData(
key: string,
data:
| Array<
HashMapValue | Array<HashMapValue> | HashMap<HashMapValue> | undefined
>
| HashMap<
HashMapValue | Array<HashMapValue> | HashMap<HashMapValue> | undefined
>
): this {
public setSampleData(key: string, data: Array<any> | HashMap<any>): this {
const clientConfig = BaseClient.config.data
if (!key || !data) return this
if (key == "params" && !clientConfig.sendParams) return this
Expand Down

0 comments on commit 4a498be

Please sign in to comment.