Skip to content

Commit 57397d9

Browse files
authored
fix: stop indexer-agent logs emitting duplicate JSON keys (#1239)
The indexer-agent tags every log line with the service name, but a few lines reuse that same label to also carry a subgraph name, so the label appears twice and log tooling silently keeps only one copy. This gives the subgraph name its own label and adds a lint rule that blocks the whole class of duplicate-key mistakes going forward.
1 parent 256919b commit 57397d9

9 files changed

Lines changed: 73 additions & 40 deletions

File tree

packages/indexer-agent/.eslintrc.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,18 @@ module.exports = {
22
root: true,
33
parser: '@typescript-eslint/parser',
44
plugins: ['@typescript-eslint'],
5-
extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'prettier']
5+
extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'prettier'],
6+
rules: {
7+
// Pino emits a per-call field whose key matches a logger binding as a SECOND JSON
8+
// key; strict parsers keep only the last. Forbid pino reserved keys as per-call fields.
9+
'no-restricted-syntax': [
10+
'error',
11+
{
12+
selector:
13+
"CallExpression[callee.property.name=/^(trace|debug|info|warn|error|fatal)$/] > ObjectExpression > Property[key.name=/^(name|level|time|pid|hostname|msg|v)$/]",
14+
message:
15+
'Do not pass a pino reserved key (name, level, time, pid, hostname, msg, v) as a per-call log field: it collides with the logger bindings and emits a duplicate JSON key that strict parsers silently drop. Use a distinct key (e.g. subgraphName) or set bindings via logger.child({ ... }).',
16+
},
17+
],
18+
},
619
}

packages/indexer-agent/src/agent.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -954,7 +954,7 @@ export class Agent {
954954
const name = `indexer-agent/${deployment.ipfsHash.slice(-10)}`
955955

956956
logger.info(`Index subgraph deployment`, {
957-
name,
957+
subgraphName: name,
958958
deployment: deployment.display,
959959
})
960960

packages/indexer-agent/src/db/cli/umzug.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const sequelize = new Sequelize({
5050
logging: false,
5151
})
5252

53-
logger.debug('Successfully connected to DB', { name: database })
53+
logger.debug('Successfully connected to DB', { database })
5454

5555
export const migrator = new Umzug({
5656
migrations: {

packages/indexer-cli/.eslintrc.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,18 @@ module.exports = {
22
root: false,
33
parser: '@typescript-eslint/parser',
44
plugins: ['@typescript-eslint'],
5-
extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'prettier']
5+
extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'prettier'],
6+
rules: {
7+
// Pino emits a per-call field whose key matches a logger binding as a SECOND JSON
8+
// key; strict parsers keep only the last. Forbid pino reserved keys as per-call fields.
9+
'no-restricted-syntax': [
10+
'error',
11+
{
12+
selector:
13+
"CallExpression[callee.property.name=/^(trace|debug|info|warn|error|fatal)$/] > ObjectExpression > Property[key.name=/^(name|level|time|pid|hostname|msg|v)$/]",
14+
message:
15+
'Do not pass a pino reserved key (name, level, time, pid, hostname, msg, v) as a per-call log field: it collides with the logger bindings and emits a duplicate JSON key that strict parsers silently drop. Use a distinct key (e.g. subgraphName) or set bindings via logger.child({ ... }).',
16+
},
17+
],
18+
},
619
}

packages/indexer-common/.eslintrc.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,18 @@ module.exports = {
22
root: false,
33
parser: '@typescript-eslint/parser',
44
plugins: ['@typescript-eslint'],
5-
extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'prettier']
5+
extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'prettier'],
6+
rules: {
7+
// Pino emits a per-call field whose key matches a logger binding as a SECOND JSON
8+
// key; strict parsers keep only the last. Forbid pino reserved keys as per-call fields.
9+
'no-restricted-syntax': [
10+
'error',
11+
{
12+
selector:
13+
"CallExpression[callee.property.name=/^(trace|debug|info|warn|error|fatal)$/] > ObjectExpression > Property[key.name=/^(name|level|time|pid|hostname|msg|v)$/]",
14+
message:
15+
'Do not pass a pino reserved key (name, level, time, pid, hostname, msg, v) as a per-call log field: it collides with the logger bindings and emits a duplicate JSON key that strict parsers silently drop. Use a distinct key (e.g. subgraphName) or set bindings via logger.child({ ... }).',
16+
},
17+
],
18+
},
619
}

packages/indexer-common/src/graph-node.ts

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -536,16 +536,16 @@ export class GraphNode {
536536

537537
async create(name: string): Promise<void> {
538538
try {
539-
this.logger.info(`Create subgraph name`, { name })
539+
this.logger.info(`Create subgraph name`, { subgraphName: name })
540540
const response = await this.admin.request('subgraph_create', { name })
541541
if (response.error) {
542542
throw response.error
543543
}
544-
this.logger.info(`Successfully created subgraph name`, { name })
544+
this.logger.info(`Successfully created subgraph name`, { subgraphName: name })
545545
} catch (error) {
546546
if (error.message.includes('already exists')) {
547547
this.logger.debug(`Subgraph name already exists, will deploy to existing name`, {
548-
name,
548+
subgraphName: name,
549549
})
550550
return
551551
}
@@ -556,7 +556,7 @@ export class GraphNode {
556556
async deploy(name: string, deployment: SubgraphDeploymentID): Promise<void> {
557557
try {
558558
this.logger.info(`Deploy subgraph deployment`, {
559-
name,
559+
subgraphName: name,
560560
deployment: deployment.display,
561561
})
562562
const response = await this.admin.request('subgraph_deploy', {
@@ -566,15 +566,15 @@ export class GraphNode {
566566

567567
this.logger.trace(`Response from 'subgraph_deploy' call`, {
568568
deployment: deployment.display,
569-
name,
569+
subgraphName: name,
570570
response,
571571
})
572572

573573
if (response.error) {
574574
throw response.error
575575
}
576576
this.logger.info(`Successfully deployed subgraph deployment`, {
577-
name,
577+
subgraphName: name,
578578
deployment: deployment.display,
579579
})
580580
} catch (error) {
@@ -587,7 +587,7 @@ export class GraphNode {
587587

588588
const err = indexerError(errorCode, error)
589589
this.logger.error(INDEXER_ERROR_MESSAGES[errorCode], {
590-
name,
590+
subgraphName: name,
591591
deployment: deployment.display,
592592
err,
593593
})
@@ -678,7 +678,7 @@ export class GraphNode {
678678
currentAssignments?: SubgraphDeploymentAssignment[],
679679
): Promise<void> {
680680
this.logger.debug('Ensure subgraph deployment is syncing', {
681-
name,
681+
subgraphName: name,
682682
deployment: deployment.ipfsHash,
683683
})
684684
try {
@@ -693,12 +693,12 @@ export class GraphNode {
693693

694694
if (matchingAssignment?.paused == false) {
695695
this.logger.debug('Subgraph deployment already syncing, ensure() is a no-op', {
696-
name,
696+
subgraphName: name,
697697
deployment: deployment.ipfsHash,
698698
})
699699
} else if (matchingAssignment?.paused == true) {
700700
this.logger.debug('Subgraph deployment paused, resuming', {
701-
name,
701+
subgraphName: name,
702702
deployment: deployment.ipfsHash,
703703
})
704704
await this.resume(deployment)
@@ -714,7 +714,7 @@ export class GraphNode {
714714
this.logger.debug(
715715
'Subgraph deployment not found, creating subgraph name and deploying...',
716716
{
717-
name,
717+
subgraphName: name,
718718
deployment: deployment.ipfsHash,
719719
},
720720
)
@@ -725,7 +725,7 @@ export class GraphNode {
725725
if (!(error instanceof IndexerError)) {
726726
const errorCode = IndexerErrorCode.IE020
727727
this.logger.error(INDEXER_ERROR_MESSAGES[errorCode], {
728-
name,
728+
subgraphName: name,
729729
deployment: deployment.display,
730730
error: indexerError(errorCode, error),
731731
})
@@ -753,7 +753,7 @@ export class GraphNode {
753753
// Safety check - should not happen if called correctly from ensure()
754754
if (!this.manifestResolver) {
755755
this.logger.error('Auto-graft called but manifest resolver not initialized', {
756-
name,
756+
subgraphName: name,
757757
deployment: deployment.display,
758758
})
759759
return
@@ -763,7 +763,7 @@ export class GraphNode {
763763
const dependencies = await this.manifestResolver.resolveWithDependencies(deployment)
764764
if (dependencies.dependencies.length == 0) {
765765
this.logger.debug('No subgraph dependencies found', {
766-
name,
766+
subgraphName: name,
767767
deployment: deployment.display,
768768
})
769769
} else {
@@ -788,7 +788,7 @@ export class GraphNode {
788788

789789
if (dependencyAssignment) {
790790
this.logger.info("Dependency subgraph found, checking if it's healthy", {
791-
name,
791+
subgraphName: name,
792792
deployment: dependency.base.display,
793793
block_required: dependency.block,
794794
})
@@ -940,11 +940,9 @@ export class GraphNode {
940940
throw new Error(`Chain not found in indexing status for deployment`)
941941
}
942942

943-
// NOTES:
944-
// - latestBlock is the latest block that has been indexed
945-
// - earliestBlock and chainHeadBlock are the earliest and latest blocks on the chain, respectively
946-
// if the deployment is paused and latestBlock is null or lower than we need, unpause it,
947-
// otherwise, if it's paused, we can't unpause it, so just wait
943+
// Unpause a paused deployment only when its indexed head (latestBlock) is null
944+
// or below the block we need; once paused past that point it can't be resumed,
945+
// so wait instead.
948946
if (
949947
deployed[0].paused &&
950948
(!chain.latestBlock || chain.latestBlock.number < blockHeight)

packages/indexer-common/src/indexer-management/monitor.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,8 @@ export class NetworkMonitor {
164164
* @returns network `alias` if the network is supported, `null` otherwise
165165
*/
166166
async allocationNetworkAlias(allocation: Allocation): Promise<string | null> {
167-
// TODO:
168-
// resolveChainId will throw an Error when we can't resolve the chainId in
169-
// the future, let's get this from the epoch subgraph (perhaps at startup)
170-
// and then resolve it here.
167+
// TODO: resolveChainId will throw when we can't resolve the chainId; in the future
168+
// get this from the epoch subgraph (perhaps at startup) and resolve it here.
171169
try {
172170
const { network: allocationNetworkAlias } = await this.graphNode.subgraphFeatures(
173171
allocation.subgraphDeployment.id,
@@ -947,7 +945,7 @@ Please submit an issue at https://github.com/graphprotocol/block-oracle/issues/n
947945
} else {
948946
this.logger.error(`Failed to query latest epoch number`, {
949947
err,
950-
msg: err.message,
948+
errorMessage: err.message,
951949
networkID,
952950
networkAlias,
953951
})
@@ -1388,10 +1386,9 @@ Please submit an issue at https://github.com/graphprotocol/block-oracle/issues/n
13881386
return [hexlify(new Uint8Array(32).fill(0)), 0]
13891387
}
13901388

1391-
// poi = undefined, force=true -- submit even if poi is 0x0
1392-
// poi = defined, force=true -- no generatedPOI needed, just submit the POI supplied (with some sanitation?)
1393-
// poi = undefined, force=false -- submit with generated POI if one available
1394-
// poi = defined, force=false -- submit user defined POI only if generated POI matches
1389+
// force=true: poi undefined -> submit even if 0x0; poi defined -> submit the supplied POI
1390+
// force=false: poi undefined -> submit a generated POI if available; poi defined -> submit the
1391+
// user POI only if it matches the generated POI
13951392
switch (force) {
13961393
case true:
13971394
switch (!!poi) {

packages/indexer-common/src/network.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export class Network {
125125
networkProvider,
126126
specification.subgraphs.maxBlockDistance,
127127
specification.subgraphs.freshnessSleepMilliseconds,
128-
logger.child({ component: 'FreshnessChecker' }),
128+
logger.child({ subComponent: 'FreshnessChecker' }),
129129
Infinity,
130130
)
131131

@@ -162,7 +162,7 @@ export class Network {
162162
networkProvider,
163163
specification.subgraphs.maxBlockDistance,
164164
specification.subgraphs.freshnessSleepMilliseconds,
165-
logger.child({ component: 'FreshnessChecker' }),
165+
logger.child({ subComponent: 'FreshnessChecker' }),
166166
Infinity,
167167
)
168168
indexingPaymentsSubgraph = await SubgraphClient.create({
@@ -214,7 +214,7 @@ export class Network {
214214
networkProvider,
215215
specification.subgraphs.maxBlockDistance,
216216
specification.subgraphs.freshnessSleepMilliseconds,
217-
logger.child({ component: 'FreshnessChecker' }),
217+
logger.child({ subComponent: 'FreshnessChecker' }),
218218
Infinity,
219219
)
220220

@@ -243,8 +243,7 @@ export class Network {
243243
contracts,
244244
specification.indexerOptions,
245245
logger.child({
246-
component: 'NetworkMonitor',
247-
protocolNetwork: specification.networkIdentifier,
246+
subComponent: 'NetworkMonitor',
248247
}),
249248
graphNode,
250249
networkSubgraph,

packages/indexer-common/src/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export async function monitorEthBalance(
4646
metrics: Metrics,
4747
networkIdentifier: string,
4848
): Promise<void> {
49-
logger = logger.child({ component: 'ETHBalanceMonitor' })
49+
logger = logger.child({ subComponent: 'ETHBalanceMonitor' })
5050

5151
logger.info('Monitor operator ETH balance (refreshes every 120s)')
5252

0 commit comments

Comments
 (0)