Skip to content

Commit

Permalink
fix: remove invoker attributes from jsx scope (#194)
Browse files Browse the repository at this point in the history
* fix: remove invoker attributes from jsx scope

* fix: install new ibm telemetry attributes major
  • Loading branch information
francinelucca committed Mar 8, 2024
1 parent 1e9aded commit 6c2d122
Show file tree
Hide file tree
Showing 8 changed files with 6 additions and 228 deletions.
8 changes: 4 additions & 4 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"devDependencies": {
"@commitlint/cli": "^17.8.1",
"@commitlint/config-conventional": "^17.8.1",
"@ibm/telemetry-attributes-js": "^2.1.0",
"@ibm/telemetry-attributes-js": "^3.0.0",
"@ibm/telemetry-config-schema": "^0.3.0",
"@opentelemetry/api": "^1.7.0",
"@opentelemetry/exporter-metrics-otlp-http": "^0.48.0",
Expand Down
2 changes: 0 additions & 2 deletions src/main/scopes/jsx/jsx-element-accumulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ export class JsxElementAccumulator {
public readonly imports: JsxImport[]
public readonly elements: JsxElement[]
public readonly elementImports: Map<JsxElement, JsxImport>
public readonly elementInvokers: Map<JsxElement, string>

constructor() {
this.imports = []
this.elements = []
this.elementImports = new Map()
this.elementInvokers = new Map()
}
}
31 changes: 1 addition & 30 deletions src/main/scopes/jsx/jsx-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,25 +243,16 @@ export class JsxScope extends Scope {
this.processFile(accumulator, sourceFile)
this.removeIrrelevantImports(accumulator, instrumentedPackage.name)
this.resolveElementImports(accumulator, importMatchers)
await this.resolveInvokers(accumulator, sourceFile.fileName)

accumulator.elements.forEach((jsxElement) => {
const jsxImport = accumulator.elementImports.get(jsxElement)
const invoker = accumulator.elementInvokers.get(jsxElement)

if (jsxImport === undefined) {
return
}

this.capture(
new ElementMetric(
jsxElement,
jsxImport,
invoker,
instrumentedPackage,
this.config,
this.logger
)
new ElementMetric(jsxElement, jsxImport, instrumentedPackage, this.config, this.logger)
)
})
}
Expand Down Expand Up @@ -310,26 +301,6 @@ export class JsxScope extends Scope {
})
}

/**
* Adds data to the accumulator for each package that invokes the jsx elements in the accumulator.
*
* @param accumulator - Accumulator to store results in.
* @param sourceFilePath - Absolute path to a sourceFile.
*/
async resolveInvokers(accumulator: JsxElementAccumulator, sourceFilePath: string) {
const containingDir = await getDirectoryPrefix(path.dirname(sourceFilePath), this.logger)

if (containingDir === undefined) {
return
}

const containingPackageData = await getPackageData(containingDir, this.root, this.logger)

accumulator.elements.forEach((jsxElement) => {
accumulator.elementInvokers.set(jsxElement, containingPackageData.name)
})
}

/**
* **For testing purposes only.**
* Makes the JsxScope collection run "synchronously" (one source file at a time). Defaults to
Expand Down
18 changes: 0 additions & 18 deletions src/main/scopes/jsx/metrics/element-metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export class ElementMetric extends ScopeMetric {
public override name = 'jsx.element' as const
private readonly jsxElement: JsxElement
private readonly matchingImport: JsxImport
private readonly invoker: string | undefined
private readonly allowedAttributeNames: string[]
private readonly allowedAttributeStringValues: string[]
private readonly instrumentedPackage: PackageData
Expand All @@ -34,23 +33,20 @@ export class ElementMetric extends ScopeMetric {
*
* @param jsxElement - Object containing name and version to extract data to generate metric from.
* @param matchingImport - Import that matched the provided JsxElement in the file.
* @param invoker - Name of the package that invoked the JsxElement.
* @param instrumentedPackage - Data (name and version) pertaining to instrumented package.
* @param config - Determines which attributes name and values to collect for.
* @param logger - Logger instance.
*/
public constructor(
jsxElement: JsxElement,
matchingImport: JsxImport,
invoker: string | undefined,
instrumentedPackage: PackageData,
config: ConfigSchema,
logger: Logger
) {
super(logger)
this.jsxElement = jsxElement
this.matchingImport = matchingImport
this.invoker = invoker
this.instrumentedPackage = instrumentedPackage

this.allowedAttributeNames = config.collect.jsx?.elements?.allowedAttributeNames ?? []
Expand All @@ -64,14 +60,6 @@ export class ElementMetric extends ScopeMetric {
* @returns OpenTelemetry compliant attributes, anonymized and substituted where necessary.
*/
public override get attributes(): Attributes {
let invokingPackageDetails

if (this.invoker !== undefined) {
invokingPackageDetails = new PackageDetailsProvider(this.logger).getPackageDetails(
this.invoker
)
}

const attrMap = deNull(
this.jsxElement.attributes.reduce<Record<string, JsxElementAttribute['value']>>(
(prev, cur) => {
Expand Down Expand Up @@ -108,9 +96,6 @@ export class ElementMetric extends ScopeMetric {
[JsxScopeAttributes.ATTRIBUTE_VALUES]: Object.values(anonymizedAttributes).map((attr) =>
String(attr)
),
[JsxScopeAttributes.INVOKER_PACKAGE_RAW]: this.invoker,
[JsxScopeAttributes.INVOKER_PACKAGE_OWNER]: invokingPackageDetails?.owner,
[JsxScopeAttributes.INVOKER_PACKAGE_NAME]: invokingPackageDetails?.name,
[NpmScopeAttributes.INSTRUMENTED_RAW]: this.instrumentedPackage.name,
[NpmScopeAttributes.INSTRUMENTED_OWNER]: instrumentedOwner,
[NpmScopeAttributes.INSTRUMENTED_NAME]: instrumentedName,
Expand All @@ -130,9 +115,6 @@ export class ElementMetric extends ScopeMetric {
}

metricData = hash(metricData, [
JsxScopeAttributes.INVOKER_PACKAGE_RAW,
JsxScopeAttributes.INVOKER_PACKAGE_OWNER,
JsxScopeAttributes.INVOKER_PACKAGE_NAME,
NpmScopeAttributes.INSTRUMENTED_RAW,
NpmScopeAttributes.INSTRUMENTED_OWNER,
NpmScopeAttributes.INSTRUMENTED_NAME,
Expand Down
8 changes: 0 additions & 8 deletions src/test/scopes/jsx/__snapshots__/jsx-scope.e2e.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,6 @@ exports[`class: JsxScope > run > captures metrics for workspace files when instr
"[redacted3]",
"[redacted5]",
],
"jsx.element.invoker.package.name": "73893d30923f338108486f1a6388bac31603db30e1b954a1ab6a77b1ab9d148d",
"jsx.element.invoker.package.raw": "73893d30923f338108486f1a6388bac31603db30e1b954a1ab6a77b1ab9d148d",
"jsx.element.module.specifier": "instrumented-top-level",
"jsx.element.name": "PackageComp",
"npm.dependency.instrumented.name": "460da73f7e63aeb72106788158261156d165376633de24e4a6673122ce2470aa",
Expand Down Expand Up @@ -219,8 +217,6 @@ exports[`class: JsxScope > run > captures metrics when instrumented package is i
"[redacted3]",
"[redacted5]",
],
"jsx.element.invoker.package.name": "5937e73b5c979346b88f6dbdccf669c11ad505927df9a05557d336a15d1d7433",
"jsx.element.invoker.package.raw": "5937e73b5c979346b88f6dbdccf669c11ad505927df9a05557d336a15d1d7433",
"jsx.element.module.specifier": "instrumented",
"jsx.element.name": "PackageComp",
"npm.dependency.instrumented.name": "ec46e364d52dab2e207d33617267a84df5838926793c1b0a2974899fe28229f1",
Expand Down Expand Up @@ -322,8 +318,6 @@ exports[`class: JsxScope > run > correctly captures jsx element metric data 1`]
"[redacted3]",
"[redacted5]",
],
"jsx.element.invoker.package.name": "e497ed4e62a3b47d9cd8aaebefb6c0837b94fb8d710461ba203aa33ccf64cc4d",
"jsx.element.invoker.package.raw": "e497ed4e62a3b47d9cd8aaebefb6c0837b94fb8d710461ba203aa33ccf64cc4d",
"jsx.element.module.specifier": "instrumented",
"jsx.element.name": "[Default]",
"npm.dependency.instrumented.name": "ec46e364d52dab2e207d33617267a84df5838926793c1b0a2974899fe28229f1",
Expand Down Expand Up @@ -365,8 +359,6 @@ exports[`class: JsxScope > run > correctly captures jsx element metric data 1`]
"[redacted13]",
"[redacted15]",
],
"jsx.element.invoker.package.name": "e497ed4e62a3b47d9cd8aaebefb6c0837b94fb8d710461ba203aa33ccf64cc4d",
"jsx.element.invoker.package.raw": "e497ed4e62a3b47d9cd8aaebefb6c0837b94fb8d710461ba203aa33ccf64cc4d",
"jsx.element.module.specifier": "instrumented",
"jsx.element.name": "ImaginaryThing",
"npm.dependency.instrumented.name": "ec46e364d52dab2e207d33617267a84df5838926793c1b0a2974899fe28229f1",
Expand Down
34 changes: 0 additions & 34 deletions src/test/scopes/jsx/jsx-scope.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,38 +370,4 @@ describe('class: JsxScope', () => {
}).not.toThrow()
})
})

describe('resolveInvokers', () => {
const jsxScope = new JsxScope('', '', config, logger)
const element1 = {
name: 'elName',
prefix: undefined,
raw: '',
attributes: []
}
const element2 = {
name: 'elName2',
prefix: undefined,
raw: '',
attributes: []
}

// TODO: This test currently fails because of this bug:
// https://github.com/nodejs/node/issues/47928
// This could be solved by mocking runCommand to return a hard-coded value, but this is less
// than useful for an end-to-end test which strives to hit as much of the underlying
// infrastructure as possible.
// eslint-disable-next-line vitest/no-disabled-tests -- See above
it.skip('correctly sets invoker name for elements', async () => {
const fileName = new Fixture('projects/basic-project/test.jsx')
const accumulator = new JsxElementAccumulator()
accumulator.elements.push(element1)
accumulator.elements.push(element2)

await jsxScope.resolveInvokers(accumulator, fileName.path)

expect(accumulator.elementInvokers.get(element1)).toBe('basic-project')
expect(accumulator.elementInvokers.get(element2)).toBe('basic-project')
})
})
})
Loading

0 comments on commit 6c2d122

Please sign in to comment.