Skip to content

Commit

Permalink
fix: add additional logging
Browse files Browse the repository at this point in the history
  • Loading branch information
jharvey10 committed Oct 26, 2023
1 parent 599fc70 commit cd20350
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 58 deletions.
9 changes: 2 additions & 7 deletions src/main/core/log/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class Logger {
* @param args - The arguments passed to the method/function.
*/
public traceEnter(targetName: string, methodName: string, args: unknown[]) {
const stringArgs = truncateString(String(args.map(safeStringify)), MAX_ARGS_STRING_LENGTH)
const stringArgs = String(args.map(safeStringify))

this.debug(`-> ${targetName}::${methodName}(${stringArgs})`)
}
Expand Down Expand Up @@ -122,12 +122,7 @@ export class Logger {
this.error(`-x- ${targetName}::${methodName}(...): ${safeStringify(result)}`)
this.error(result)
} else {
this.debug(
`<- ${targetName}::${methodName}(...): ${truncateString(
safeStringify(result),
MAX_ARGS_STRING_LENGTH
)}`
)
this.debug(`<- ${targetName}::${methodName}(...): ${safeStringify(result)}`)
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/main/core/scope-metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
*/
import { type Attributes } from '@opentelemetry/api'

import { Loggable } from './log/loggable.js'

/**
* An extendable base class for all scope metrics. Scope metrics are required to define a name and
* a method which returns an OpenTelemetry-compatible Attributes object.
*/
export abstract class ScopeMetric {
export abstract class ScopeMetric extends Loggable {
/**
* The OpenTelemetry-style name of this metric to be used in data transfer and storage.
*/
Expand Down
17 changes: 14 additions & 3 deletions src/main/scopes/npm/find-installers-from-tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
* LICENSE file in the root directory of this source tree.
*/

import { type Logger } from '../../core/log/logger.js'
import { findNestedDeps } from './find-nested-deps.js'
import { getPackageSubTree } from './get-package-sub-tree.js'
import { type InstallingPackage } from './interfaces.js'

/*
* Nested dependency considerations:
Expand Down Expand Up @@ -36,20 +38,29 @@ import { getPackageSubTree } from './get-package-sub-tree.js'
* @param dependencyTree - The tree to search.
* @param packageName - The name of the package for which to search.
* @param packageVersion - The specific version of the package for which to search.
* @param logger - A logger instance.
* @returns An array of results.
*/
export function findInstallersFromTree(
dependencyTree: Record<string, unknown>,
packageName: string,
packageVersion: string
packageVersion: string,
logger: Logger
) {
logger.traceEnter('find-installers-from-tree', 'findInstallersFromTree', [
dependencyTree,
packageName,
packageVersion
])
// Matches come back as something like: [..., parentPkgName, dependencies, instrumentedPackage]
const matches = findNestedDeps(dependencyTree, packageName, packageVersion)
let result: InstallingPackage[] = []

if (matches.length >= 1) {
// We want to ignore last 2 pieces to get the parent's info, not the child's
return matches.map((match) => getPackageSubTree(dependencyTree, match.slice(0, -2)))
result = matches.map((match) => getPackageSubTree(dependencyTree, match.slice(0, -2)))
}

return []
logger.traceExit('find-installers-from-tree', 'findInstallersFromTree', result)
return result
}
8 changes: 6 additions & 2 deletions src/main/scopes/npm/metrics/dependency-metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { type Attributes } from '@opentelemetry/api'
import { SemVer } from 'semver'

import { anonymize } from '../../../core/anonymize.js'
import { type Logger } from '../../../core/log/logger.js'
import { Trace } from '../../../core/log/trace.js'
import { ScopeMetric } from '../../../core/scope-metric.js'

export interface DependencyData {
Expand All @@ -29,9 +31,10 @@ export class DependencyMetric extends ScopeMetric {
* Constructs a DependencyMetric.
*
* @param data - Object containing name and version to extract data to generate metric from.
* @param logger - The logger instance to use.
*/
public constructor(data: DependencyData) {
super()
public constructor(data: DependencyData, logger: Logger) {
super(logger)
this.name = 'dependency.count'
this.data = data
}
Expand Down Expand Up @@ -93,6 +96,7 @@ export class DependencyMetric extends ScopeMetric {
* @param rawPackageVersion - Raw version of package.
* @returns Object containing package owner, name, major, minor, patch and preRelease versions.
*/
@Trace()
private getPackageDetails(rawPackageName: string, rawPackageVersion: string) {
let owner, name

Expand Down
17 changes: 10 additions & 7 deletions src/main/scopes/npm/npm-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,15 @@ export class NpmScope extends Scope {
installingPackages.forEach((installingPkg) => {
installingPkg.dependencies.forEach((dependency) => {
this.capture(
new DependencyMetric({
name: dependency.name,
version: dependency.version,
installerName: installingPkg.name,
installerVersion: installingPkg.version
})
new DependencyMetric(
{
name: dependency.name,
version: dependency.version,
installerName: installingPkg.name,
installerVersion: installingPkg.version
},
this.logger
)
)
})
})
Expand Down Expand Up @@ -99,7 +102,7 @@ export class NpmScope extends Scope {

const dependencyTree = JSON.parse(result.stdout)

installers = findInstallersFromTree(dependencyTree, packageName, packageVersion)
installers = findInstallersFromTree(dependencyTree, packageName, packageVersion, this.logger)

if (installers.length > 0) {
break
Expand Down
2 changes: 1 addition & 1 deletion src/test/core/scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('scope', () => {
public override get attributes(): Attributes {
return { hello: 'hi' }
}
})()
})(logger)

expect(myScope.metrics['my-metric']).not.toBeDefined()

Expand Down
23 changes: 17 additions & 6 deletions src/test/scopes/npm/find-installers-from-tree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,42 @@
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/
import { describe, expect, it } from 'vitest'
import { afterAll, describe, expect, it } from 'vitest'

import { createLogFilePath } from '../../../main/core/log/create-log-file-path.js'
import { Logger } from '../../../main/core/log/logger.js'
import { findInstallersFromTree } from '../../../main/scopes/npm/find-installers-from-tree.js'
import { Fixture } from '../../__utils/fixture.js'

const testDependencyTree = await new Fixture('dependency-trees/basic-dependency-tree.json').parse()
const logger = new Logger(await createLogFilePath(new Date().toISOString()))

describe('findInstallersFromTree', () => {
afterAll(async () => {
await logger.close()
})

it('returns empty array if dependency does not exist', () => {
expect(findInstallersFromTree(testDependencyTree, 'not-there', '1.0.0')).toStrictEqual([])
expect(findInstallersFromTree(testDependencyTree, 'not-there', '1.0.0', logger)).toStrictEqual(
[]
)
})

it('returns for a single installer', () => {
expect(findInstallersFromTree(testDependencyTree, 'two', '1.0.0')).toMatchSnapshot()
expect(findInstallersFromTree(testDependencyTree, 'two', '1.0.0', logger)).toMatchSnapshot()
})

it('returns for multiple installers', () => {
expect(findInstallersFromTree(testDependencyTree, 'three', '1.0.0')).toMatchSnapshot()
expect(findInstallersFromTree(testDependencyTree, 'three', '1.0.0', logger)).toMatchSnapshot()
})

it('only picks up correct version', () => {
expect(findInstallersFromTree(testDependencyTree, 'four', '1.0.1')).toMatchSnapshot()
expect(findInstallersFromTree(testDependencyTree, 'four', '1.0.1', logger)).toMatchSnapshot()
})

it('disregards other versions', () => {
expect(findInstallersFromTree(testDependencyTree, 'four', 'not-there')).toStrictEqual([])
expect(findInstallersFromTree(testDependencyTree, 'four', 'not-there', logger)).toStrictEqual(
[]
)
})
})
84 changes: 53 additions & 31 deletions src/test/scopes/npm/metrics/dependency-metric.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,29 @@
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/
import { describe, expect, it } from 'vitest'
import { afterAll, describe, expect, it } from 'vitest'

import { anonymize } from '../../../../main/core/anonymize.js'
import { createLogFilePath } from '../../../../main/core/log/create-log-file-path.js'
import { Logger } from '../../../../main/core/log/logger.js'
import { DependencyMetric } from '../../../../main/scopes/npm/metrics/dependency-metric.js'

const logger = new Logger(await createLogFilePath(new Date().toISOString()))

describe('dependencyMetric', () => {
afterAll(async () => {
await logger.close()
})
it('returns the correct attributes for a standard package', () => {
const attributes = new DependencyMetric({
name: 'test-1',
version: '0.0.1',
installerName: 'test-1-installer',
installerVersion: '1.0.0'
}).attributes
const attributes = new DependencyMetric(
{
name: 'test-1',
version: '0.0.1',
installerName: 'test-1-installer',
installerVersion: '1.0.0'
},
logger
).attributes
expect(attributes).toStrictEqual(
anonymize(
{
Expand Down Expand Up @@ -56,12 +66,15 @@ describe('dependencyMetric', () => {
})

it('returns the correct attributes for a package with a prerelease', () => {
const attributes = new DependencyMetric({
name: 'test-1',
version: '0.0.1-rc.0',
installerName: 'test-1-installer',
installerVersion: '1.0.0-rc.4'
}).attributes
const attributes = new DependencyMetric(
{
name: 'test-1',
version: '0.0.1-rc.0',
installerName: 'test-1-installer',
installerVersion: '1.0.0-rc.4'
},
logger
).attributes
expect(attributes).toStrictEqual(
anonymize(
{
Expand Down Expand Up @@ -101,12 +114,15 @@ describe('dependencyMetric', () => {
})

it('returns the correct attributes for a package with metadata', () => {
const attributes = new DependencyMetric({
name: 'test-1',
version: '0.0.1+12345',
installerName: 'test-1-installer',
installerVersion: '1.0.0+9999'
}).attributes
const attributes = new DependencyMetric(
{
name: 'test-1',
version: '0.0.1+12345',
installerName: 'test-1-installer',
installerVersion: '1.0.0+9999'
},
logger
).attributes
expect(attributes).toStrictEqual(
anonymize(
{
Expand Down Expand Up @@ -146,12 +162,15 @@ describe('dependencyMetric', () => {
})

it('returns the correct attributes for a package with a prerelease and metadata', () => {
const attributes = new DependencyMetric({
name: 'test-1',
version: '0.0.1-rc.1+12345',
installerName: 'test-1-installer',
installerVersion: '1.0.0-rc.0+99999'
}).attributes
const attributes = new DependencyMetric(
{
name: 'test-1',
version: '0.0.1-rc.1+12345',
installerName: 'test-1-installer',
installerVersion: '1.0.0-rc.0+99999'
},
logger
).attributes

expect(attributes).toStrictEqual(
anonymize(
Expand Down Expand Up @@ -192,12 +211,15 @@ describe('dependencyMetric', () => {
})

it('returns the correct attributes for a package with an owner', () => {
const attributes = new DependencyMetric({
name: '@owner/test-1',
version: '0.0.1-rc.0+12345',
installerName: '@installer/test-1-installer',
installerVersion: '1.0.0'
}).attributes
const attributes = new DependencyMetric(
{
name: '@owner/test-1',
version: '0.0.1-rc.0+12345',
installerName: '@installer/test-1-installer',
installerVersion: '1.0.0'
},
logger
).attributes
expect(attributes).toStrictEqual(
anonymize(
{
Expand Down

0 comments on commit cd20350

Please sign in to comment.