Merge pull request #512 from bugsnag/release/v2.10.0

Release v2.10.0

Author: gingerbenw

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [v2.10.0] (2024-09-26)
+
+### Added
+
+- Allow setting custom span attributes [#510](https://github.com/bugsnag/bugsnag-js-performance/pull/510)
+
 ## [v2.9.1] (2024-09-11)
 
 ### Fixed

CONTRIBUTING.md

@@ -0,0 +1,39 @@
+# Contributing
+
+Thanks for stopping by! This document should cover most topics surrounding contributing to this repo.
+
+* [How to contribute](#how-to-contribute)
+  * [Reporting issues](#reporting-issues)
+  * [Fixing issues](#fixing-issues)
+  * [Adding features](#adding-features)
+
+## Reporting issues
+Are you having trouble getting started? Please [contact us directly](mailto:support@bugsnag.com?subject=%5BGitHub%5D%20bugsnag-js-performance%20-%20having%20trouble%20getting%20started%20with%20BugSnag) for assistance with integrating BugSnag into your application.
+If you have spotted a problem with this module, feel free to open a [new issue](https://github.com/bugsnag/bugsnag-js-performance/issues/new?template=Bug_report.md). Here are a few things to check before doing so:
+
+* Are you using the latest version of BugSnag? If not, does updating to the latest version fix your issue?
+* Has somebody else [already reported](https://github.com/bugsnag/bugsnag-js-performance/issues?utf8=%E2%9C%93&q=is%3Aissue%20is%3Aopen) your issue? Feel free to add additional context to or check-in on an existing issue that matches your own.
+* Is your issue caused by this module? Only things related to `@bugsnag/browser-performance`, `@bugsnag/react-native-performance`, and performance related plugins (such as `@bugsnag/plugin-react-navigation-performance`) should be reported here. For anything else, please [contact us directly](mailto:support@bugsnag.com) and we'd be happy to help you out.
+
+### Fixing issues
+
+If you've identified a fix to a new or existing issue, we welcome contributions!
+Here are some helpful suggestions on contributing that help us merge your PR quickly and smoothly:
+
+* [Fork](https://help.github.com/articles/fork-a-repo) the
+  [library on GitHub](https://github.com/bugsnag/bugsnag-js-performance)
+* Build and test your changes. You can use `npm pack` to build the module locally and install it in a real app.
+* Commit and push until you are happy with your contribution
+* [Make a pull request](https://help.github.com/articles/using-pull-requests)
+
+### Adding features
+
+Unfortunately we’re unable to accept PRs that add features or refactor the library at this time.
+However, we’re very eager and welcome to hearing feedback about the library so please contact us directly to discuss your idea, or open a
+[feature request](https://github.com/bugsnag/bugsnag-js-performance/issues/new?template=Feature_request.md) to help us improve the library.
+
+Here’s a bit about our process designing and building the BugSnag libraries:
+
+* We have an internal roadmap to plan out the features we build, and sometimes we will already be planning your suggested feature!
+* Our open source libraries span many languages and frameworks so we strive to ensure they are idiomatic on the given platform, but also consistent in terminology between platforms. That way the core concepts are familiar whether you adopt BugSnag for one platform or many.
+* Finally, one of our goals is to ensure our libraries work reliably, even in crashy, multi-threaded environments. Oftentimes, this requires an intensive engineering design and code review process that adheres to our style and linting guidelines.

bin/generate-react-native-fixture

@@ -50,7 +50,7 @@ const PACKAGE_DIRECTORIES = [
 ]
 
 // make sure we install a compatible versions of peer dependencies
-const reactNativeFileAccessVersion = parseFloat(reactNativeVersion) <= 0.64 ? '1.7.1' : '3.1.0'
+const reactNativeFileAccessVersion = parseFloat(reactNativeVersion) <= 0.64 ? '1.7.1' : '3.1.1'
 const netinfoVersion = parseFloat(reactNativeVersion) <= 0.64 ? '10.0.0' : '11.3.2'
 const DEPENDENCIES = [
   `@bugsnag/react-native@${process.env.NOTIFIER_VERSION}`,
@@ -110,15 +110,6 @@ if (!process.env.SKIP_GENERATE_FIXTURE) {
     configureRN064Fixture(fixtureDir)
   }
 
-  // due to a bug in react-native-file-access, static frameworks is required on 0.75+ othwerwise iOS builds will fail.
-  // there is an open PR to fix this upstream: https://github.com/alpha0010/react-native-file-access/pull/88
-  // TODO: remove this when either the upstream PR is released or PLAT-12626 (replace react-native-file-access with our own File I/O) is implemented
-  if (parseFloat(reactNativeVersion) >= 0.75) {
-    let podfileContents = fs.readFileSync(`${fixtureDir}/ios/Podfile`, 'utf8')
-    podfileContents = podfileContents.replace(/target 'reactnative' do/, 'use_frameworks! :linkage => :static\ntarget \'reactnative\' do')
-    fs.writeFileSync(`${fixtureDir}/ios/Podfile`, podfileContents)
-  }
-
   // link react-native-navigation using rnn-link tool
   if (process.env.REACT_NATIVE_NAVIGATION === 'true' || process.env.REACT_NATIVE_NAVIGATION === '1') {
     execSync('npx rnn-link', { cwd: fixtureDir, stdio: 'inherit' })

packages/core/lib/attributes.ts

@@ -1,23 +1,106 @@
-import type { Configuration, InternalConfiguration } from './config'
+import type { Configuration, InternalConfiguration, Logger } from './config'
+import { ATTRIBUTE_KEY_LENGTH_LIMIT, defaultResourceAttributeLimits } from './custom-attribute-limits'
 import type { SpanInternal } from './span'
 import { isNumber } from './validation'
 
-export type SpanAttribute = string | number | boolean
+interface StringAttributeValue { stringValue: string }
+interface IntAttributeValue { intValue: string }
+interface DoubleAttributeValue { doubleValue: number }
+interface BoolAttributeValue { boolValue: boolean }
+
+type SingleAttributeValue = StringAttributeValue | IntAttributeValue | DoubleAttributeValue | BoolAttributeValue
+
+interface ArrayAttributeValue {
+  arrayValue: {
+    values?: SingleAttributeValue[]
+  }
+}
+
+type JsonAttributeValue = SingleAttributeValue | ArrayAttributeValue
+
+export interface JsonAttribute {
+  key: string
+  value: JsonAttributeValue
+}
+
+type Attribute = string | number | boolean
+
+// Array values should always be of the same type, although the trace server will accept mixed types
+type ArrayAttribute = string[] | number[] | boolean[]
+
+export type SpanAttribute = Attribute | ArrayAttribute
 
 export interface SpanAttributesSource <C extends Configuration> {
   configure: (configuration: InternalConfiguration<C>) => void
   requestAttributes: (span: SpanInternal) => void
 }
 
+export interface SpanAttributesLimits {
+  attributeStringValueLimit: number
+  attributeArrayLengthLimit: number
+  attributeCountLimit: number
+}
+
+function truncateString (value: string, limit: number) {
+  const originalLength = value.length
+  const newString = value.slice(0, limit)
+  const truncatedLength = newString.length
+
+  return `${newString} *** ${originalLength - truncatedLength} CHARS TRUNCATED`
+}
+
 export class SpanAttributes {
   private readonly attributes: Map<string, SpanAttribute>
+  private readonly logger: Logger
+  private readonly spanAttributeLimits: SpanAttributesLimits
+  private readonly spanName: string
+  private _droppedAttributesCount = 0
+
+  get droppedAttributesCount () {
+    return this._droppedAttributesCount
+  }
 
-  constructor (initialValues: Map<string, SpanAttribute>) {
+  constructor (initialValues: Map<string, SpanAttribute>, spanAttributeLimits: SpanAttributesLimits, spanName: string, logger: Logger) {
     this.attributes = initialValues
+    this.spanAttributeLimits = spanAttributeLimits
+    this.spanName = spanName
+    this.logger = logger
+  }
+
+  private validateAttribute (name: string, value: SpanAttribute) {
+    if (typeof value === 'string' && value.length > this.spanAttributeLimits.attributeStringValueLimit) {
+      this.attributes.set(name, truncateString(value, this.spanAttributeLimits.attributeStringValueLimit))
+      this.logger.warn(`Span attribute ${name} in span ${this.spanName} was truncated as the string exceeds the ${this.spanAttributeLimits.attributeStringValueLimit} character limit set by attributeStringValueLimit.`)
+    }
+
+    if (Array.isArray(value) && value.length > this.spanAttributeLimits.attributeArrayLengthLimit) {
+      const truncatedValue = value.slice(0, this.spanAttributeLimits.attributeArrayLengthLimit)
+      this.attributes.set(name, truncatedValue)
+      this.logger.warn(`Span attribute ${name} in span ${this.spanName} was truncated as the array exceeds the ${this.spanAttributeLimits.attributeArrayLengthLimit} element limit set by attributeArrayLengthLimit.`)
+    }
   }
 
   set (name: string, value: SpanAttribute) {
-    if (typeof value === 'string' || typeof value === 'boolean' || isNumber(value)) {
+    if (typeof name === 'string' && (typeof value === 'string' || typeof value === 'boolean' || isNumber(value) || Array.isArray(value))) {
+      this.attributes.set(name, value)
+    }
+  }
+
+  // Used by the public API to set custom attributes
+  setCustom (name: string, value: SpanAttribute) {
+    if (typeof name === 'string' && (typeof value === 'string' || typeof value === 'boolean' || isNumber(value) || Array.isArray(value))) {
+      if (!this.attributes.has(name) && this.attributes.size >= this.spanAttributeLimits.attributeCountLimit) {
+        this._droppedAttributesCount++
+        this.logger.warn(`Span attribute ${name} in span ${this.spanName} was dropped as the number of attributes exceeds the ${this.spanAttributeLimits.attributeCountLimit} attribute limit set by attributeCountLimit.`)
+        return
+      }
+
+      if (name.length > ATTRIBUTE_KEY_LENGTH_LIMIT) {
+        this._droppedAttributesCount++
+        this.logger.warn(`Span attribute ${name} in span ${this.spanName} was dropped as the key length exceeds the ${ATTRIBUTE_KEY_LENGTH_LIMIT} character fixed limit.`)
+        return
+      }
+
       this.attributes.set(name, value)
     }
   }
@@ -27,12 +110,13 @@ export class SpanAttributes {
   }
 
   toJson () {
+    Array.from(this.attributes).forEach(([key, value]) => { this.validateAttribute(key, value) })
     return Array.from(this.attributes).map(([key, value]) => attributeToJson(key, value))
   }
 }
 
 export class ResourceAttributes extends SpanAttributes {
-  constructor (releaseStage: string, appVersion: string, serviceName: string, sdkName: string, sdkVersion: string) {
+  constructor (releaseStage: string, appVersion: string, serviceName: string, sdkName: string, sdkVersion: string, logger: Logger) {
     const initialValues = new Map([
       ['deployment.environment', releaseStage],
       ['telemetry.sdk.name', sdkName],
@@ -44,21 +128,49 @@ export class ResourceAttributes extends SpanAttributes {
       initialValues.set('service.version', appVersion)
     }
 
-    super(initialValues)
+    // TODO: this class should be refactored to use a common base class instead of SpanAttributes
+    // since we don't need a span name and logger for resource attributes - see PLAT-12820
+    super(initialValues, defaultResourceAttributeLimits, 'resource-attributes', logger)
   }
 }
 
 export type ResourceAttributeSource<C extends Configuration>
   = (configuration: InternalConfiguration<C>) => Promise<ResourceAttributes>
 
-export interface JsonAttribute {
-  key: string
-  value: { stringValue: string }
-  | { intValue: string }
-  | { doubleValue: number }
-  | { boolValue: boolean }
+function getJsonAttributeValue (value: Attribute): SingleAttributeValue | undefined {
+  switch (typeof value) {
+    case 'number':
+      if (Number.isNaN(value) || !Number.isFinite(value)) {
+        return undefined
+      }
+
+      if (Number.isInteger(value)) {
+        return { intValue: `${value}` }
+      }
+
+      return { doubleValue: value }
+    case 'boolean':
+      return { boolValue: value }
+    case 'string':
+      return { stringValue: value }
+    default:
+      // Ensure all JsonAttributeValue cases are handled
+      value satisfies never
+  }
+}
+
+function getJsonArrayAttributeValue (attributeArray: Attribute[]): SingleAttributeValue[] {
+  return attributeArray
+    .map((value) => getJsonAttributeValue(value))
+    .filter(value => typeof value !== 'undefined')
 }
 
+/**
+ * Converts a span attribute into an OTEL compliant value i.e. { stringValue: 'value' }
+ * @param key the name of the span attribute
+ * @param attribute the value of the attribute. Can be of type string | number | boolean | string[] | number[] | boolean[]. Invalid types will be removed from array attributes.
+ * @returns
+ */
 export function attributeToJson (key: string, attribute: SpanAttribute): JsonAttribute | undefined {
   switch (typeof attribute) {
     case 'number':
@@ -76,6 +188,12 @@ export function attributeToJson (key: string, attribute: SpanAttribute): JsonAtt
       return { key, value: { boolValue: attribute } }
     case 'string':
       return { key, value: { stringValue: attribute } }
+    case 'object':
+      if (Array.isArray(attribute)) {
+        const arrayValues = getJsonArrayAttributeValue(attribute)
+        return { key, value: { arrayValue: arrayValues.length > 0 ? { values: arrayValues } : {} } }
+      }
+      return undefined
     default:
       return undefined
   }

packages/core/lib/batch-processor.ts

@@ -4,7 +4,13 @@ import type ProbabilityManager from './probability-manager'
 import type { Processor } from './processor'
 import type { RetryQueue } from './retry-queue'
 import type { ReadonlySampler } from './sampler'
-import type { SpanEnded } from './span'
+import type { Span, SpanEnded } from './span'
+
+import { millisecondsToNanoseconds } from './clock'
+import { spanEndedToSpan } from './span'
+
+export type OnSpanEndCallback = (span: Span) => boolean | Promise<boolean>
+export type OnSpanEndCallbacks = OnSpanEndCallback[]
 
 type MinimalProbabilityManager = Pick<ProbabilityManager, 'setProbability' | 'ensureFreshProbability'>
 
@@ -108,6 +114,37 @@ export class BatchProcessor<C extends Configuration> implements Processor {
     await this.flushQueue
   }
 
+  private async runCallbacks (span: Span): Promise<boolean> {
+    if (this.configuration.onSpanEnd) {
+      const callbackStartTime = performance.now()
+      let continueToBatch = true
+      for (const callback of this.configuration.onSpanEnd) {
+        try {
+          let result = callback(span)
+
+          // @ts-expect-error result may or may not be a promise
+          if (typeof result.then === 'function') {
+            result = await result
+          }
+
+          if (result === false) {
+            continueToBatch = false
+            break
+          }
+        } catch (err) {
+          this.configuration.logger.error('Error in onSpanEnd callback: ' + err)
+        }
+      }
+      if (continueToBatch) {
+        const duration = millisecondsToNanoseconds(performance.now() - callbackStartTime)
+        span.setAttribute('bugsnag.span.callbacks_duration', duration)
+      }
+      return continueToBatch
+    } else {
+      return true
+    }
+  }
+
   private async prepareBatch (): Promise<SpanEnded[] | undefined> {
     if (this.spans.length === 0) {
       return
@@ -126,7 +163,10 @@ export class BatchProcessor<C extends Configuration> implements Processor {
       }
 
       if (this.sampler.sample(span)) {
-        batch.push(span)
+        // Run any callbacks that have been registered before batching
+        // as callbacks could cause the span to be discarded
+        const shouldAddToBatch = await this.runCallbacks(spanEndedToSpan(span))
+        if (shouldAddToBatch) batch.push(span)
       }
     }
 

packages/core/lib/config.ts

@@ -1,5 +1,14 @@
+import type { OnSpanEndCallbacks } from './batch-processor'
+import {
+  ATTRIBUTE_ARRAY_LENGTH_LIMIT_DEFAULT,
+  ATTRIBUTE_COUNT_LIMIT_DEFAULT,
+  ATTRIBUTE_STRING_VALUE_LIMIT_DEFAULT,
+  ATTRIBUTE_ARRAY_LENGTH_LIMIT_MAX,
+  ATTRIBUTE_COUNT_LIMIT_MAX,
+  ATTRIBUTE_STRING_VALUE_LIMIT_MAX
+} from './custom-attribute-limits'
 import type { Plugin } from './plugin'
-import { isLogger, isNumber, isObject, isPluginArray, isString, isStringArray, isStringWithLength } from './validation'
+import { isLogger, isNumber, isObject, isOnSpanEndCallbacks, isPluginArray, isString, isStringArray, isStringWithLength } from './validation'
 
 type SetTraceCorrelation = (traceId: string, spanId: string) => void
 
@@ -40,6 +49,10 @@ export interface Configuration {
   plugins?: Array<Plugin<Configuration>>
   bugsnag?: BugsnagErrorStatic
   samplingProbability?: number
+  onSpanEnd?: OnSpanEndCallbacks
+  attributeStringValueLimit?: number
+  attributeArrayLengthLimit?: number
+  attributeCountLimit?: number
 }
 
 export interface TestConfiguration {
@@ -120,6 +133,26 @@ export const schema: CoreSchema = {
     defaultValue: undefined,
     message: 'should be a number between 0 and 1',
     validate: (value: unknown): value is number | undefined => value === undefined || (isNumber(value) && value >= 0 && value <= 1)
+  },
+  onSpanEnd: {
+    defaultValue: undefined,
+    message: 'should be an array of functions',
+    validate: isOnSpanEndCallbacks
+  },
+  attributeStringValueLimit: {
+    defaultValue: ATTRIBUTE_STRING_VALUE_LIMIT_DEFAULT,
+    message: `should be a number between 1 and ${ATTRIBUTE_STRING_VALUE_LIMIT_MAX}`,
+    validate: (value: unknown): value is number => isNumber(value) && value > 0 && value <= ATTRIBUTE_STRING_VALUE_LIMIT_MAX
+  },
+  attributeArrayLengthLimit: {
+    defaultValue: ATTRIBUTE_ARRAY_LENGTH_LIMIT_DEFAULT,
+    message: `should be a number between 1 and ${ATTRIBUTE_ARRAY_LENGTH_LIMIT_MAX}`,
+    validate: (value: unknown): value is number => isNumber(value) && value > 0 && value <= ATTRIBUTE_ARRAY_LENGTH_LIMIT_MAX
+  },
+  attributeCountLimit: {
+    defaultValue: ATTRIBUTE_COUNT_LIMIT_DEFAULT,
+    message: `should be a number between 1 and ${ATTRIBUTE_COUNT_LIMIT_MAX}`,
+    validate: (value: unknown): value is number => isNumber(value) && value > 0 && value <= ATTRIBUTE_COUNT_LIMIT_MAX
   }
 }