fix(generator): handle two InetAddress indexes sharing one InetAddressType - #1666
Open
pujitha24 wants to merge 1 commit into
Open
fix(generator): handle two InetAddress indexes sharing one InetAddressType#1666pujitha24 wants to merge 1 commit into
pujitha24 wants to merge 1 commit into
Conversation
…sType
Motivation:
Some MIB tables use a single InetAddressType field to qualify two (or
more) subsequent InetAddress-typed index columns in the same row, e.g.
CISCO-IETF-IPMROUTE-MIB's cIpMRouteEntry:
INDEX { cIpMRouteAddrType, cIpMRouteGroup, cIpMRouteSource, cIpMRouteSourceMask }
Generating a module that walks metrics from this table panics:
panic: runtime error: slice bounds out of range [:-1]
main.generateConfigModule.func2(...)
generator/tree.go:415
Approach:
The index-collapsing logic in generateConfigModule combines a
(TypeField, AddressField) pair into a single index, and separately
(added in prometheus#782) collapses a 3-field (Afi, Safi, Address) pattern by
dropping the two preceding fields. When a second InetAddress field
immediately follows a first one that already consumed the shared
preceding InetAddressType, the code incorrectly fell into the
Afi/Safi/Address branch and tried to remove two elements from a slice
that only had one left.
The fix distinguishes "the preceding field is itself a sibling
combined-address value that already dropped the shared type" (nothing
left to remove) from the genuine Afi/Safi/Address case (drop two
fields), instead of always assuming the latter whenever the type is
found two positions back.
Validation:
Added a unit test in generator/tree_test.go covering a table with one
InetAddressType index followed by two InetAddress indexes, asserting
both become separate InetAddress-typed index labels without panicking.
Reproduced the original panic locally against the real
CISCO-IETF-IPMROUTE-MIB from the issue and confirmed the generator now
produces a snmp.yml with cIpMRouteGroup/cIpMRouteSource as separate
InetAddress indexes instead of crashing.
Ran:
go test ./generator/... -run TestGenerateConfigModule -v (PASS, 3/3 subtests)
go build ./... (OK)
go test ./... (all packages OK)
This only fixes the panic and the resulting metric loss for tables with
this index shape; it does not change behavior for tables the generator
already handled correctly.
Fixes prometheus#1218
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Some MIB tables use a single InetAddressType field to qualify two (or
more) subsequent InetAddress-typed index columns in the same row, e.g.
CISCO-IETF-IPMROUTE-MIB's cIpMRouteEntry:
Generating a module that walks metrics from this table panics:
Approach:
The index-collapsing logic in generateConfigModule combines a
(TypeField, AddressField) pair into a single index, and separately
(added in #782) collapses a 3-field (Afi, Safi, Address) pattern by
dropping the two preceding fields. When a second InetAddress field
immediately follows a first one that already consumed the shared
preceding InetAddressType, the code incorrectly fell into the
Afi/Safi/Address branch and tried to remove two elements from a slice
that only had one left.
The fix distinguishes "the preceding field is itself a sibling
combined-address value that already dropped the shared type" (nothing
left to remove) from the genuine Afi/Safi/Address case (drop two
fields), instead of always assuming the latter whenever the type is
found two positions back.
Validation:
Added a unit test in generator/tree_test.go covering a table with one
InetAddressType index followed by two InetAddress indexes, asserting
both become separate InetAddress-typed index labels without panicking.
Reproduced the original panic locally against the real
CISCO-IETF-IPMROUTE-MIB from the issue and confirmed the generator now
produces a snmp.yml with cIpMRouteGroup/cIpMRouteSource as separate
InetAddress indexes instead of crashing.
Ran:
go test ./generator/... -run TestGenerateConfigModule -v (PASS, 3/3 subtests)
go build ./... (OK)
go test ./... (all packages OK)
This only fixes the panic and the resulting metric loss for tables with
this index shape; it does not change behavior for tables the generator
already handled correctly.
Fixes #1218
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com