Skip to content

Commit

Permalink
Changed log level for retryable error Scenarios (#22141)
Browse files Browse the repository at this point in the history
* Changed log level :
    1- xDS DeltaDiscoveryRequest context cancelled set to INFO on stream cancelled as it is retrieable
    2- subscription failed logging on ACL change is set to INFO as is also retriable

* Added change log file and updated code for PR comments

* fixing error logging with key, value, updating changelog
  • Loading branch information
anandmukul93 authored Feb 12, 2025
1 parent a7eb703 commit e82b93b
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 2 deletions.
6 changes: 6 additions & 0 deletions .changelog/22141.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
```release-note:improvement
xDS: Log level change from ERROR to INFO for xDS delta discovery request. Stream can be cancelled on server shutdown and other scenarios. It is retryable and error is a superfluous log.
```
```release-note:improvement
SubMatView: Log level change from ERROR to INFO for subject materialized view as subscription creation is retryable on ACL change.
```
9 changes: 8 additions & 1 deletion agent/submatview/materializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ package submatview

import (
"context"
"errors"
"sync"
"time"

"github.com/hashicorp/go-hclog"

"github.com/hashicorp/consul/agent/consul/stream"
"github.com/hashicorp/consul/lib/retry"
"github.com/hashicorp/consul/proto/private/pbsubscribe"
)
Expand Down Expand Up @@ -202,7 +204,12 @@ func (m *materializer) handleError(req *pbsubscribe.SubscribeRequest, err error)
logger = logger.With("key", req.Key) // nolint:staticcheck // SA1019 intentional use of deprecated field
}

logger.Error("subscribe call failed")
switch {
case errors.Is(err, stream.ErrACLChanged):
logger.Info("subscribe call failed due to ACL change", "error", err)
default:
logger.Error("subscribe call failed", "error", err)
}
}

// isNonTemporaryOrConsecutiveFailure returns true if the error is not a
Expand Down
7 changes: 6 additions & 1 deletion agent/xds/delta.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,12 @@ func (s *Server) DeltaAggregatedResources(stream ADSDeltaStream) error {
return
}
if err != nil {
s.Logger.Error("Error receiving new DeltaDiscoveryRequest; closing request channel", "error", err)
switch {
case status.Code(err) == codes.Canceled:
s.Logger.Info("Error receiving new DeltaDiscoveryRequest; closing request channel", "error", err)
default:
s.Logger.Error("Error receiving new DeltaDiscoveryRequest", "error", err)
}
close(reqCh)
return
}
Expand Down

0 comments on commit e82b93b

Please sign in to comment.