Skip to content

Commit 52af2a5

Browse files
committed
add monorail recording
1 parent b576023 commit 52af2a5

6 files changed

Lines changed: 174 additions & 0 deletions

File tree

packages/app/src/cli/commands/app/config/validate.test.ts

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import {testAppLinked} from '../../../models/app/app.test-data.js'
55
import {Project} from '../../../models/project/project.js'
66
import {selectActiveConfig} from '../../../models/project/active-config.js'
77
import {errorsForConfig} from '../../../models/project/config-selection.js'
8+
import metadata from '../../../metadata.js'
89
import {outputResult} from '@shopify/cli-kit/node/output'
10+
import {AbortError} from '@shopify/cli-kit/node/error'
911
import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
1012
import {describe, expect, test, vi} from 'vitest'
1113

@@ -14,12 +16,19 @@ vi.mock('../../../services/validate.js')
1416
vi.mock('../../../models/project/project.js')
1517
vi.mock('../../../models/project/active-config.js')
1618
vi.mock('../../../models/project/config-selection.js')
19+
vi.mock('../../../metadata.js', () => ({default: {addPublicMetadata: vi.fn()}}))
1720
vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
1821
const actual = await importOriginal<typeof import('@shopify/cli-kit/node/output')>()
1922
return {...actual, outputResult: vi.fn()}
2023
})
2124
vi.mock('@shopify/cli-kit/node/ui')
2225

26+
async function expectValidationMetadataCalls(...expectedMetadata: Record<string, unknown>[]) {
27+
const metadataCalls = vi.mocked(metadata.addPublicMetadata).mock.calls.map(([getMetadata]) => getMetadata)
28+
expect(metadataCalls).toHaveLength(expectedMetadata.length)
29+
await expect(Promise.all(metadataCalls.map((getMetadata) => getMetadata()))).resolves.toEqual(expectedMetadata)
30+
}
31+
2332
function mockHealthyProject() {
2433
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
2534
vi.mocked(selectActiveConfig).mockResolvedValue({file: new TomlFile('shopify.app.toml', {})} as any)
@@ -36,6 +45,7 @@ describe('app config validate command', () => {
3645
await Validate.run([], import.meta.url)
3746

3847
expect(validateApp).toHaveBeenCalledWith(app, {json: false})
48+
await expectValidationMetadataCalls({cmd_app_validate_json: false})
3949
})
4050

4151
test('calls validateApp with json: true when --json flag is passed', async () => {
@@ -47,6 +57,7 @@ describe('app config validate command', () => {
4757
await Validate.run(['--json'], import.meta.url)
4858

4959
expect(validateApp).toHaveBeenCalledWith(app, {json: true})
60+
await expectValidationMetadataCalls({cmd_app_validate_json: true})
5061
})
5162

5263
test('calls validateApp with json: true when -j flag is passed', async () => {
@@ -58,6 +69,7 @@ describe('app config validate command', () => {
5869
await Validate.run(['-j'], import.meta.url)
5970

6071
expect(validateApp).toHaveBeenCalledWith(app, {json: true})
72+
await expectValidationMetadataCalls({cmd_app_validate_json: true})
6173
})
6274

6375
test('outputs JSON issues when active config has TOML parse errors', async () => {
@@ -71,5 +83,88 @@ describe('app config validate command', () => {
7183

7284
expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
7385
expect(linkedAppContext).not.toHaveBeenCalled()
86+
await expectValidationMetadataCalls(
87+
{cmd_app_validate_json: true},
88+
{
89+
cmd_app_validate_valid: false,
90+
cmd_app_validate_issue_count: 1,
91+
cmd_app_validate_file_count: 1,
92+
},
93+
)
94+
})
95+
96+
test('records failure metadata for config errors in non-json mode', async () => {
97+
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
98+
vi.mocked(selectActiveConfig).mockResolvedValue({file: new TomlFile('shopify.app.toml', {})} as any)
99+
vi.mocked(errorsForConfig).mockReturnValue([
100+
{path: '/app/shopify.app.toml', message: 'Missing required field'} as any,
101+
{path: '/app/shopify.app.toml', message: 'Invalid value'} as any,
102+
])
103+
104+
await expect(Validate.run([], import.meta.url)).rejects.toThrow()
105+
106+
expect(linkedAppContext).not.toHaveBeenCalled()
107+
await expectValidationMetadataCalls(
108+
{cmd_app_validate_json: false},
109+
{
110+
cmd_app_validate_valid: false,
111+
cmd_app_validate_issue_count: 2,
112+
cmd_app_validate_file_count: 1,
113+
},
114+
)
115+
})
116+
117+
test('records failure metadata when Project.load fails with --json', async () => {
118+
vi.mocked(Project.load).mockRejectedValue(new AbortError('Could not find app configuration'))
119+
120+
await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()
121+
122+
expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
123+
expect(selectActiveConfig).not.toHaveBeenCalled()
124+
await expectValidationMetadataCalls(
125+
{cmd_app_validate_json: true},
126+
{
127+
cmd_app_validate_valid: false,
128+
cmd_app_validate_issue_count: 1,
129+
cmd_app_validate_file_count: 1,
130+
},
131+
)
132+
})
133+
134+
test('records failure metadata when selectActiveConfig fails with --json', async () => {
135+
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
136+
vi.mocked(selectActiveConfig).mockRejectedValue(new AbortError('No config found'))
137+
138+
await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()
139+
140+
expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
141+
expect(linkedAppContext).not.toHaveBeenCalled()
142+
await expectValidationMetadataCalls(
143+
{cmd_app_validate_json: true},
144+
{
145+
cmd_app_validate_valid: false,
146+
cmd_app_validate_issue_count: 1,
147+
cmd_app_validate_file_count: 1,
148+
},
149+
)
150+
})
151+
152+
test('records failure metadata when linkedAppContext throws a validation error with --json', async () => {
153+
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
154+
vi.mocked(selectActiveConfig).mockResolvedValue({file: new TomlFile('shopify.app.toml', {})} as any)
155+
vi.mocked(errorsForConfig).mockReturnValue([])
156+
vi.mocked(linkedAppContext).mockRejectedValue(new AbortError('Validation errors in /app/shopify.app.toml'))
157+
158+
await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()
159+
160+
expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
161+
await expectValidationMetadataCalls(
162+
{cmd_app_validate_json: true},
163+
{
164+
cmd_app_validate_valid: false,
165+
cmd_app_validate_issue_count: 1,
166+
cmd_app_validate_file_count: 1,
167+
},
168+
)
74169
})
75170
})

packages/app/src/cli/commands/app/config/validate.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,20 @@ import {linkedAppContext} from '../../../services/app-context.js'
55
import {selectActiveConfig} from '../../../models/project/active-config.js'
66
import {errorsForConfig} from '../../../models/project/config-selection.js'
77
import {Project} from '../../../models/project/project.js'
8+
import metadata from '../../../metadata.js'
89
import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
910
import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error'
1011
import {outputResult, stringifyMessage, unstyled} from '@shopify/cli-kit/node/output'
1112
import {renderError} from '@shopify/cli-kit/node/ui'
1213

14+
async function recordValidationFailure(issueCount: number, fileCount: number) {
15+
await metadata.addPublicMetadata(() => ({
16+
cmd_app_validate_valid: false,
17+
cmd_app_validate_issue_count: issueCount,
18+
cmd_app_validate_file_count: fileCount,
19+
}))
20+
}
21+
1322
export default class Validate extends AppLinkedCommand {
1423
static summary = 'Validate your app configuration and extensions.'
1524

@@ -26,12 +35,17 @@ export default class Validate extends AppLinkedCommand {
2635
public async run(): Promise<AppLinkedCommandOutput> {
2736
const {flags} = await this.parse(Validate)
2837

38+
await metadata.addPublicMetadata(() => ({
39+
cmd_app_validate_json: flags.json,
40+
}))
41+
2942
// Stage 1: Load project
3043
let project: Project
3144
try {
3245
project = await Project.load(flags.path)
3346
} catch (err) {
3447
if (err instanceof AbortError && flags.json) {
48+
await recordValidationFailure(1, 1)
3549
const message = unstyled(stringifyMessage(err.message)).trim()
3650
outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
3751
throw new AbortSilentError()
@@ -45,6 +59,7 @@ export default class Validate extends AppLinkedCommand {
4559
activeConfig = await selectActiveConfig(project, flags.config)
4660
} catch (err) {
4761
if (err instanceof AbortError && flags.json) {
62+
await recordValidationFailure(1, 1)
4863
const message = unstyled(stringifyMessage(err.message)).trim()
4964
outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
5065
throw new AbortSilentError()
@@ -55,6 +70,8 @@ export default class Validate extends AppLinkedCommand {
5570
const configErrors = errorsForConfig(project, activeConfig.file)
5671
if (configErrors.length > 0) {
5772
const issues = configErrors.map((err) => ({file: err.path, message: err.message}))
73+
const fileCount = new Set(configErrors.map((err) => err.path)).size
74+
await recordValidationFailure(issues.length, fileCount)
5875
if (flags.json) {
5976
outputResult(JSON.stringify({valid: false, issues}, null, 2))
6077
throw new AbortSilentError()
@@ -83,6 +100,7 @@ export default class Validate extends AppLinkedCommand {
83100
const message = err instanceof AbortError ? unstyled(stringifyMessage(err.message)).trim() : ''
84101
const isValidationError = message.startsWith('Validation errors in ')
85102
if (isValidationError && flags.json) {
103+
await recordValidationFailure(1, 1)
86104
outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
87105
throw new AbortSilentError()
88106
}

packages/app/src/cli/services/validate.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import {validateApp} from './validate.js'
22
import {testAppLinked} from '../models/app/app.test-data.js'
33
import {AppErrors, formatConfigurationError} from '../models/app/loader.js'
4+
import metadata from '../metadata.js'
45
import {describe, expect, test, vi} from 'vitest'
56
import {outputResult} from '@shopify/cli-kit/node/output'
67
import {renderError, renderSuccess} from '@shopify/cli-kit/node/ui'
78
import {AbortSilentError} from '@shopify/cli-kit/node/error'
89

10+
vi.mock('../metadata.js', () => ({default: {addPublicMetadata: vi.fn()}}))
911
vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
1012
const actual = await importOriginal<typeof import('@shopify/cli-kit/node/output')>()
1113
return {
@@ -15,6 +17,16 @@ vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
1517
})
1618
vi.mock('@shopify/cli-kit/node/ui')
1719

20+
async function expectLastValidationMetadata(expected: {
21+
cmd_app_validate_valid: boolean
22+
cmd_app_validate_issue_count: number
23+
cmd_app_validate_file_count: number
24+
}) {
25+
const getMetadata = vi.mocked(metadata.addPublicMetadata).mock.calls.at(-1)?.[0]
26+
expect(getMetadata).toBeDefined()
27+
await expect(Promise.resolve(getMetadata!())).resolves.toEqual(expected)
28+
}
29+
1830
describe('formatConfigurationError', () => {
1931
test('returns plain message when no path', () => {
2032
expect(formatConfigurationError({file: 'foo.toml', message: 'something broke'})).toBe('something broke')
@@ -34,6 +46,11 @@ describe('validateApp', () => {
3446
expect(renderSuccess).toHaveBeenCalledWith({headline: 'App configuration is valid.'})
3547
expect(renderError).not.toHaveBeenCalled()
3648
expect(outputResult).not.toHaveBeenCalled()
49+
await expectLastValidationMetadata({
50+
cmd_app_validate_valid: true,
51+
cmd_app_validate_issue_count: 0,
52+
cmd_app_validate_file_count: 0,
53+
})
3754
})
3855

3956
test('outputs json success when --json is enabled and there are no errors', async () => {
@@ -42,6 +59,11 @@ describe('validateApp', () => {
4259
expect(outputResult).toHaveBeenCalledWith(JSON.stringify({valid: true, issues: []}, null, 2))
4360
expect(renderSuccess).not.toHaveBeenCalled()
4461
expect(renderError).not.toHaveBeenCalled()
62+
await expectLastValidationMetadata({
63+
cmd_app_validate_valid: true,
64+
cmd_app_validate_issue_count: 0,
65+
cmd_app_validate_file_count: 0,
66+
})
4567
})
4668

4769
test('renders errors and throws when there are validation errors', async () => {
@@ -58,6 +80,11 @@ describe('validateApp', () => {
5880
})
5981
expect(renderSuccess).not.toHaveBeenCalled()
6082
expect(outputResult).not.toHaveBeenCalled()
83+
await expectLastValidationMetadata({
84+
cmd_app_validate_valid: false,
85+
cmd_app_validate_issue_count: 2,
86+
cmd_app_validate_file_count: 2,
87+
})
6188
})
6289

6390
test('outputs structured json issues when --json is enabled and there are validation errors', async () => {
@@ -83,6 +110,11 @@ describe('validateApp', () => {
83110
)
84111
expect(renderError).not.toHaveBeenCalled()
85112
expect(renderSuccess).not.toHaveBeenCalled()
113+
await expectLastValidationMetadata({
114+
cmd_app_validate_valid: false,
115+
cmd_app_validate_issue_count: 2,
116+
cmd_app_validate_file_count: 2,
117+
})
86118
})
87119

88120
test('renders success when errors object exists but is empty', async () => {
@@ -111,5 +143,10 @@ describe('validateApp', () => {
111143
2,
112144
),
113145
)
146+
await expectLastValidationMetadata({
147+
cmd_app_validate_valid: false,
148+
cmd_app_validate_issue_count: 1,
149+
cmd_app_validate_file_count: 1,
150+
})
114151
})
115152
})

packages/app/src/cli/services/validate.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {AppLinkedInterface} from '../models/app/app.js'
22
import {formatConfigurationError} from '../models/app/loader.js'
3+
import metadata from '../metadata.js'
34
import {outputResult} from '@shopify/cli-kit/node/output'
45
import {renderError, renderSuccess} from '@shopify/cli-kit/node/ui'
56
import {AbortSilentError} from '@shopify/cli-kit/node/error'
@@ -8,10 +9,22 @@ interface ValidateAppOptions {
89
json: boolean
910
}
1011

12+
async function recordValidationMetadata(valid: boolean, errors: {file: string}[]) {
13+
const fileCount = new Set(errors.map((error) => error.file)).size
14+
15+
await metadata.addPublicMetadata(() => ({
16+
cmd_app_validate_valid: valid,
17+
cmd_app_validate_issue_count: errors.length,
18+
cmd_app_validate_file_count: fileCount,
19+
}))
20+
}
21+
1122
export async function validateApp(app: AppLinkedInterface, options: ValidateAppOptions = {json: false}): Promise<void> {
1223
const appErrors = app.errors
1324

1425
if (!appErrors || appErrors.isEmpty()) {
26+
await recordValidationMetadata(true, [])
27+
1528
if (options.json) {
1629
outputResult(JSON.stringify({valid: true, issues: []}, null, 2))
1730
return
@@ -22,6 +35,7 @@ export async function validateApp(app: AppLinkedInterface, options: ValidateAppO
2235
}
2336

2437
const errors = appErrors.getErrors()
38+
await recordValidationMetadata(false, errors)
2539

2640
if (options.json) {
2741
const issues = errors.map(({file, message, path, code}) => ({file, message, path, code}))

packages/cli-kit/src/private/node/analytics.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,12 @@ export async function getSensitiveEnvironmentData(config: Interfaces.Config) {
103103
}
104104

105105
function getShopifyEnvironmentVariables() {
106+
// Agent callers can identify themselves today via SHOPIFY_* environment
107+
// variables. The current contract is intentionally lightweight and is kept in
108+
// the sensitive payload until we prove which dimensions deserve first-class
109+
// Monorail fields, e.g. SHOPIFY_CLI_AGENT, SHOPIFY_CLI_AGENT_VERSION,
110+
// SHOPIFY_CLI_AGENT_RUN_ID, SHOPIFY_CLI_AGENT_SESSION_ID, and
111+
// SHOPIFY_CLI_AGENT_PROVIDER.
106112
return Object.fromEntries(Object.entries(process.env).filter(([key]) => key.startsWith('SHOPIFY_')))
107113
}
108114

packages/cli-kit/src/public/node/monorail.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ export interface Schemas {
8888
cmd_app_linked_config_uses_cli_managed_urls?: Optional<boolean>
8989
cmd_app_warning_api_key_deprecation_displayed?: Optional<boolean>
9090
cmd_app_deployment_mode?: Optional<string>
91+
cmd_app_validate_json?: Optional<boolean>
92+
cmd_app_validate_valid?: Optional<boolean>
93+
cmd_app_validate_issue_count?: Optional<number>
94+
cmd_app_validate_file_count?: Optional<number>
9195

9296
// Dev related commands
9397
cmd_dev_tunnel_type?: Optional<string>

0 commit comments

Comments
 (0)