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

Resolve Traps OIDs to names #10934

Merged
merged 9 commits into from
Mar 15, 2022
Merged

Resolve Traps OIDs to names #10934

merged 9 commits into from
Mar 15, 2022

Conversation

FlorianVeaux
Copy link
Member

@FlorianVeaux FlorianVeaux commented Feb 15, 2022

With this PRs traps payload format has been refactored:

  1. The source and service are now snmp-traps.
  2. The traps variables are now stored under .raw_variables instead of .variables.
  3. Traps payloads are enriched thanks to an OIDResolver interface that loads traps_db files located in snmp.d/traps_db/

How to test:

  1. If not shipped already, you can download this database file and place it under snmp.d/traps_db/ directory (you can leave it compressed).
  2. Start the agent with the following configuration in datadog.yaml
logs_enabled: true
snmp_traps_enabled: true
snmp_traps_config:
  port: 9162
  community_strings:
    - 'public'
    - 'default'
  1. Send traps on port 9162 with the command snmptrap -v2c -c public localhost:9162 '' IF-MIB::linkDown ifIndex i 12 ifAdminStatus i 1 ifOperStatus i 2
  2. Look on Datadog logs and check that the received traps payload have been correctly formatted and enriched.

@FlorianVeaux FlorianVeaux requested review from a team as code owners February 15, 2022 09:01
@FlorianVeaux FlorianVeaux changed the base branch from main to flo/traps_add_namespace February 15, 2022 09:01
@FlorianVeaux FlorianVeaux marked this pull request as draft February 15, 2022 09:01
Base automatically changed from flo/traps_add_namespace to main February 15, 2022 14:19
@FlorianVeaux FlorianVeaux added this to the 7.36.0 milestone Feb 15, 2022
@FlorianVeaux FlorianVeaux marked this pull request as ready for review February 15, 2022 15:18
@FlorianVeaux FlorianVeaux marked this pull request as draft February 16, 2022 09:25
"testing"

"github.com/gosnmp/gosnmp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var serverPort = getFreePort()

func getFreePort() uint16 {
Copy link
Member Author

Choose a reason for hiding this comment

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

Prevent flakiness

@FlorianVeaux FlorianVeaux force-pushed the flo/traps_new_format branch 2 times, most recently from 72ed2d2 to 2febe4c Compare February 16, 2022 14:32
@FlorianVeaux FlorianVeaux marked this pull request as ready for review February 16, 2022 15:37
pkg/logs/config/config.go Outdated Show resolved Hide resolved
pkg/logs/internal/launchers/traps/launcher.go Outdated Show resolved Hide resolved
pkg/snmp/traps/formatter.go Outdated Show resolved Hide resolved
pkg/snmp/traps/formatter.go Show resolved Hide resolved
Comment on lines +238 to +249
func formatVersion(packet *gosnmp.SnmpPacket) string {
switch packet.Version {
case gosnmp.Version3:
return "3"
case gosnmp.Version2c:
return "2"
case gosnmp.Version1:
return "1"
default:
return "unknown"
}
}
Copy link
Member

Choose a reason for hiding this comment

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

(no blocking for this PR since it was already there)

Do we need to report the SNMP version ?

Not sure if that's useful info, it's more related to the kind of credentials used (v2 community string, v3 user model) and not really part of the trap information. Also, a same device can support both v2 community string and v3 user model.

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't want to change it in this PR. Do you think we should remove it?
The version doesn't provide I agree but it doesn't harm to have it

Copy link
Member

Choose a reason for hiding this comment

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

pkg/snmp/traps/formatter.go Outdated Show resolved Hide resolved
pkg/snmp/traps/oid_resolver.go Outdated Show resolved Hide resolved
pkg/snmp/traps/oid_resolver.go Outdated Show resolved Hide resolved
@FlorianVeaux FlorianVeaux merged commit 0702d55 into main Mar 15, 2022
@FlorianVeaux FlorianVeaux deleted the flo/traps_new_format branch March 15, 2022 12:44
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