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.

Sign up for GitHub

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

Jump to bottom

Pass context with timeout to FQDN lookup #37756

Merged
merged 7 commits into from
Feb 2, 2024
Merged

Pass context with timeout to FQDN lookup #37756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cfc1e81
Pass context with timeout to FQDN lookup
ycombinator Jan 25, 2024
209f013
Add temporary gomod replace
ycombinator Jan 25, 2024
b145464
Add CHANGELOG entry
ycombinator Jan 25, 2024
decda55
Update dependency
ycombinator Feb 1, 2024
e5d69f2
Update method calls
ycombinator Feb 1, 2024
8855ce8
Add nolint for rand deprecation warning
ycombinator Feb 2, 2024
9f3b2ee
Make linter happy
ycombinator Feb 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ you can achieve this by overwriting the value using an `add_fields` processor. {
- Upgrade elastic-agent-libs to v0.7.5. Removes obsolete "Treating the CommonName field on X.509 certificates as a host name..." deprecation warning for 8.0. {pull}37755[37755]
- aws: Add credential caching for `AssumeRole` session tokens. {issue}37787[37787]
- Lower logging level to debug when attempting to configure beats with unknown fields from autodiscovered events/environments {pull}[37816][37816]
- Set timeout of 1 minute for FQDN requests {pull}37756[37756]
ycombinator marked this conversation as resolved.
Show resolved Hide resolved

*Auditbeat*

Expand Down
4 changes: 2 additions & 2 deletions NOTICE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14955,11 +14955,11 @@ Contents of probable licence file $GOMODCACHE/github.com/elastic/go-structform@v

--------------------------------------------------------------------------------
Dependency : github.com/elastic/go-sysinfo
Version: v1.11.2
Version: v1.12.0
Licence type (autodetected): Apache-2.0
--------------------------------------------------------------------------------

Contents of probable licence file $GOMODCACHE/github.com/elastic/go-sysinfo@v1.11.2/LICENSE.txt:
Contents of probable licence file $GOMODCACHE/github.com/elastic/go-sysinfo@v1.12.0/LICENSE.txt:


Apache License
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ require (
github.com/elastic/go-perf v0.0.0-20191212140718-9c656876f595
github.com/elastic/go-seccomp-bpf v1.4.0
github.com/elastic/go-structform v0.0.10
github.com/elastic/go-sysinfo v1.11.2
github.com/elastic/go-sysinfo v1.12.0
github.com/elastic/go-ucfg v0.8.6
github.com/elastic/gosigar v0.14.2
github.com/fatih/color v1.15.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,8 +692,8 @@ github.com/elastic/go-seccomp-bpf v1.4.0 h1:6y3lYrEHrLH9QzUgOiK8WDqmPaMnnB785Wxi
github.com/elastic/go-seccomp-bpf v1.4.0/go.mod h1:wIMxjTbKpWGQk4CV9WltlG6haB4brjSH/dvAohBPM1I=
github.com/elastic/go-structform v0.0.10 h1:oy08o/Ih2hHTkNcRY/1HhaYvIp5z6t8si8gnCJPDo1w=
github.com/elastic/go-structform v0.0.10/go.mod h1:CZWf9aIRYY5SuKSmOhtXScE5uQiLZNqAFnwKR4OrIM4=
github.com/elastic/go-sysinfo v1.11.2 h1:mcm4OSYVMyws6+n2HIVMGkln5HOpo5Ie1ZmbbNn0jg4=
github.com/elastic/go-sysinfo v1.11.2/go.mod h1:GKqR8bbMK/1ITnez9NIsIfXQr25aLhRJa7AfT8HpBFQ=
github.com/elastic/go-sysinfo v1.12.0 h1:ZKyB4N5XLnGFysNGNnJl8xvd+GBGCe2MemBykR+3yQI=
github.com/elastic/go-sysinfo v1.12.0/go.mod h1:GKqR8bbMK/1ITnez9NIsIfXQr25aLhRJa7AfT8HpBFQ=
github.com/elastic/go-ucfg v0.8.6 h1:stUeyh2goTgGX+/wb9gzKvTv0YB0231LTpKUgCKj4U0=
github.com/elastic/go-ucfg v0.8.6/go.mod h1:4E8mPOLSUV9hQ7sgLEJ4bvt0KhMuDJa8joDT2QGAEKA=
github.com/elastic/go-windows v1.0.1 h1:AlYZOldA+UJ0/2nBuqWdo90GFCgG9xuyw9SYzGUtJm0=
Expand Down
7 changes: 5 additions & 2 deletions libbeat/cmd/instance/beat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func initRand() {
} else {
seed = n.Int64()
}
rand.Seed(seed)
rand.Seed(seed) //nolint:staticcheck // need seed from cryptographically strong PRNG.
}

// Run initializes and runs a Beater implementation. name is the name of the
Expand Down Expand Up @@ -824,7 +824,10 @@ func (b *Beat) configure(settings Settings) error {
return fmt.Errorf("failed to get host information: %w", err)
}

fqdn, err := h.FQDN()
fqdnLookupCtx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
defer cancel()

fqdn, err := h.FQDNWithContext(fqdnLookupCtx)
Copy link
Member

Choose a reason for hiding this comment

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

If this times out it falls back to the non-FQDN, does that ever get updated? Or is the Beat stuck with the non-FQDN.

If the first rule of network communication is "never make a request without a timeout", the second one is "never rely on a single network request to succeed".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it doesn't get updated. I agree that it would be nice to keep retrying but I also think that's out of scope for this PR here; this one is just about implementing the first rule. I'll file a separate issue to implement the second one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#37835

if err != nil {
// FQDN lookup is "best effort". We log the error, fallback to
// the OS-reported hostname, and move on.
Expand Down
10 changes: 7 additions & 3 deletions libbeat/processors/add_host_metadata/add_host_metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
package add_host_metadata

import (
"context"
"fmt"
"sync"
"time"

"github.com/gofrs/uuid"

"github.com/elastic/elastic-agent-libs/monitoring"
"github.com/elastic/go-sysinfo"

"github.com/elastic/beats/v7/libbeat/beat"
"github.com/elastic/beats/v7/libbeat/features"
Expand All @@ -35,7 +37,6 @@ import (
"github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/elastic-agent-libs/mapstr"
"github.com/elastic/elastic-agent-system-metrics/metric/system/host"
"github.com/elastic/go-sysinfo"
)

const processorName = "add_host_metadata"
Expand Down Expand Up @@ -96,7 +97,7 @@ func New(cfg *config.C) (beat.Processor, error) {
}

// create a unique ID for this instance of the processor
cbIDStr := ""
var cbIDStr string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is unrelated to this PR; it is included in this PR to make the linter happy. Without it, the linter was failing with this error:

assigned to cbIDStr, but reassigned without using the value (wastedassign)

cbID, err := uuid.NewV4()
// if we fail, fall back to the processor name, hope for the best.
if err != nil {
Expand Down Expand Up @@ -178,7 +179,10 @@ func (p *addHostMetadata) loadData(checkCache bool, useFQDN bool) error {

hostname := h.Info().Hostname
if useFQDN {
fqdn, err := h.FQDN()
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
defer cancel()

fqdn, err := h.FQDNWithContext(ctx)
if err != nil {
// FQDN lookup is "best effort". If it fails, we monitor the failure, fallback to
// the OS-reported hostname, and move on.
Expand Down
Loading