Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expand upon example of using an index as a metric_tag #12438

Merged
merged 7 commits into from
Jul 12, 2022

Conversation

cahillsf
Copy link
Contributor

@cahillsf cahillsf commented Jul 1, 2022

What does this PR do?

Expands on our current documentation surrounding applying the index of an OID as a metric tag. Adds additional detail regarding the syntax of metric collection from a table.

Motivation

conversation with customer on this ticket and ENG on this thread

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

Copy link
Contributor

@hestonhoffman hestonhoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of style changes and then this should be good to go

docs/developer/tutorials/snmp/profile-format.md Outdated Show resolved Hide resolved
docs/developer/tutorials/snmp/profile-format.md Outdated Show resolved Hide resolved
docs/developer/tutorials/snmp/profile-format.md Outdated Show resolved Hide resolved
@cahillsf
Copy link
Contributor Author

cahillsf commented Jul 1, 2022

Hey @hestonhoffman thanks for the review! I just merged all of your suggestions when you have a chance to take a another look

hestonhoffman
hestonhoffman previously approved these changes Jul 1, 2022
Copy link
Contributor

@hestonhoffman hestonhoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@FlorianVeaux FlorianVeaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, this is very helpful!

docs/developer/tutorials/snmp/profile-format.md Outdated Show resolved Hide resolved
Comment on lines 312 to 314
The value corresponding to the `accDnForwardFilterDest` of a row in this table will be found at the following OID: `.1.3.6.1.4.1.5.1.1.19.25.1.1.<accDnForwardFilterDest>`. Retrieving the value of `.1.3.6.1.4.1.5.1.1.19.25.1.1.123` will return `123`. To capture the value `123` and apply it as a metric tag, target `index: 1`.

The value corresponding to the `accDnForwardFilterSource` of a row in this table will be found at the following OID: `.1.3.6.1.4.1.5.1.1.19.25.1.1.<accDnForwardFilterDest>.<accDnForwardFilterSource>`. Retrieving the value of `.1.3.6.1.4.1.5.1.1.19.25.1.1.123.456` will return `456`. To capture the value `456` and apply it as a metric tag, target `index: 2`.
Copy link
Member

@AlexandreYang AlexandreYang Jul 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it could more clear to illustrate with an example like this one:

metrics:
  - MIB: CISCO-FIREWALL-MIB
    table:
      OID: 1.3.6.1.4.1.9.9.147.1.2.2.2
      name: cfwConnectionStatTable
    symbols:
      - OID: 1.3.6.1.4.1.9.9.147.1.2.2.2.1.5
        name: cfwConnectionStatValue
    metric_tags:
      - index: 1 // capture first index digit
        tag: service_type
      - index: 2 // capture second index digit
        tag: stat_type

from a real example here

- MIB: CISCO-FIREWALL-MIB
table:
OID: 1.3.6.1.4.1.9.9.147.1.2.2.2
name: cfwConnectionStatTable
symbols:
- OID: 1.3.6.1.4.1.9.9.147.1.2.2.2.1.5
name: cfwConnectionStatValue
metric_tags:
- index: 1
tag: service_type
- index: 2
tag: stat_type

For example, when we fetch the value of cfwConnectionStatValue, the OID with the index is like 1.3.6.1.4.1.9.9.147.1.2.2.2.1.5.20.2 = 4087850099, here the indexes are 20.2 (1.3.6.1.4.1.9.9.147.1.2.2.2.1.5.<service type>.<stat type>).

cfwConnectionStatEntry OBJECT-TYPE
        SYNTAX     CfwConnectionStatEntry
        MAX-ACCESS not-accessible
        STATUS     current
        DESCRIPTION
            "An entry in the table, containing information about a
            firewall statistic."
        INDEX { cfwConnectionStatService, cfwConnectionStatType }
    ::= { cfwConnectionStatTable 1 }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks @AlexandreYang, pushed up a commit with these updates: 192ff22

Let me know what you think!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome -- sure thing! One more thing here @AlexandreYang it looks like I am not authorized to merge this PR - would you be able to do so on or point me in the right direction for how I can obtain authorization? Screenshot showing Merging is blocked auth message

Copy link
Member

@AlexandreYang AlexandreYang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice ! thanks a lot for improving the doc :)

@AlexandreYang AlexandreYang merged commit 2c3040f into master Jul 12, 2022
@AlexandreYang AlexandreYang deleted the cahillsf/snmp-doc-update branch July 12, 2022 13:19
github-actions bot pushed a commit that referenced this pull request Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants