-
Notifications
You must be signed in to change notification settings - Fork 78
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
Revise gRPC error statuses and details in meta (Redis/Cassandra) #1013
Revise gRPC error statuses and details in meta (Redis/Cassandra) #1013
Conversation
[CHATOPS:HELP] ChatOps commands.
|
Codecov Report
@@ Coverage Diff @@
## master #1013 +/- ##
==========================================
- Coverage 15.25% 14.04% -1.22%
==========================================
Files 498 495 -3
Lines 28770 29190 +420
==========================================
- Hits 4389 4099 -290
- Misses 24110 24835 +725
+ Partials 271 256 -15
Continue to review full report at Codecov.
|
@@ -160,10 +249,21 @@ | |||
err = s.redis.Set(ctx, kv.GetKey(), kv.GetVal()) | |||
if err != nil { | |||
log.Errorf("[SetMeta]\tunknown error\t%+v", err) | |||
err = status.WrapWithInternal(fmt.Sprintf("SetMeta API failed to store: key %s val %s", kv.GetKey(), kv.GetVal()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 121 characters (lll)
2bf54a4
to
f341215
Compare
default: | ||
log.Errorf("[GetMetasInverse]\tunknown error\t%+v", err) | ||
err = status.WrapWithUnknown(fmt.Sprintf("GetMetasInverse API: unknown error occurred: vals %#v", vals.GetVals()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 122 characters (lll)
@@ -200,10 +325,21 @@ func (s *server) SetMeta(ctx context.Context, kv *payload.Meta_KeyVal) (_ *paylo | |||
err = s.cassandra.Set(kv.GetKey(), kv.GetVal()) | |||
if err != nil { | |||
log.Errorf("[SetMeta]\tunknown error\t%+v", err) | |||
err = status.WrapWithInternal(fmt.Sprintf("SetMeta API: failed to store: key %s val %s", kv.GetKey(), kv.GetVal()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 122 characters (lll)
case errors.IsErrCassandraUnavailable(err): | ||
log.Warnf("[DeleteMetas]\tunavailable\t%+v", err) | ||
err = status.WrapWithUnavailable(fmt.Sprintf("DeleteMetas API: Cassandra unavailable: keys %#v", keys.GetKeys()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 121 characters (lll)
case errors.IsErrCassandraUnavailable(err): | ||
log.Warnf("[DeleteMetasInverse]\tunavailable\t%+v", err) | ||
err = status.WrapWithUnavailable(fmt.Sprintf("DeleteMetasInverse API: Cassandra unavailable: vals %#v", vals.GetVals()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 128 characters (lll)
case errors.IsErrCassandraUnavailable(err): | ||
log.Warnf("[GetMetasInverse]\tunavailable\t%+v", err) | ||
err = status.WrapWithUnavailable(fmt.Sprintf("GetMetasInverse API: Cassandra unavailable: vals %#v", vals.GetVals()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 125 characters (lll)
default: | ||
log.Errorf("[DeleteMetasInverse]\tunknown error\t%+v", err) | ||
err = status.WrapWithUnknown(fmt.Sprintf("DeleteMetasInverse API: unknown error occurred: vals %#v", vals.GetVals()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 125 characters (lll)
@@ -56,24 +57,55 @@ func (s *server) GetMeta(ctx context.Context, key *payload.Meta_Key) (*payload.M | |||
switch { | |||
case errors.IsErrCassandraNotFound(err): | |||
log.Warnf("[GetMeta]\tnot found\t%v\t%s", key.GetKey(), err.Error()) | |||
err = status.WrapWithNotFound(fmt.Sprintf("GetMeta API: not found: key %s", key.GetKey()), err, | |||
&errdetails.RequestInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in RequestInfo (exhaustivestruct)
RequestId: key.GetKey(), | ||
ServingData: errdetails.Serialize(key), | ||
}, | ||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
ResourceName, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
case errors.IsErrCassandraUnavailable(err): | ||
log.Warnf("[GetMeta]\tunavailable\t%+v", err) | ||
err = status.WrapWithUnavailable(fmt.Sprintf("GetMeta API: Cassandra unavailable: key %s", key.GetKey()), err, | ||
&errdetails.RequestInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in RequestInfo (exhaustivestruct)
RequestId: key.GetKey(), | ||
ServingData: errdetails.Serialize(key), | ||
}, | ||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
ResourceName, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
@@ -94,24 +126,56 @@ func (s *server) GetMetas(ctx context.Context, keys *payload.Meta_Keys) (mv *pay | |||
switch { | |||
case errors.IsErrCassandraNotFound(err): | |||
log.Warnf("[GetMetas]\tnot found\t%v\t%s", keys.GetKeys(), err.Error()) | |||
err = status.WrapWithNotFound(fmt.Sprintf("GetMetas API: not found: keys %#v", keys.GetKeys()), err, | |||
&errdetails.RequestInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
RequestId, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in RequestInfo (exhaustivestruct)
case errors.IsErrCassandraUnavailable(err): | ||
log.Warnf("[GetMetas]\tunavailable\t%+v", err) | ||
err = status.WrapWithUnavailable(fmt.Sprintf("GetMetas API: Cassandra unavailable: keys %#v", keys.GetKeys()), err, | ||
&errdetails.RequestInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
RequestId, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in RequestInfo (exhaustivestruct)
|
||
default: | ||
log.Errorf("[GetMetas]\tunknown error\t%+v", err) | ||
err = status.WrapWithUnknown(fmt.Sprintf("GetMetas API: unknown error occurred: keys %#v", keys.GetKeys()), err, | ||
&errdetails.RequestInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
RequestId, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in RequestInfo (exhaustivestruct)
case errors.IsErrCassandraUnavailable(err): | ||
log.Warnf("[DeleteMetaInverse]\tunavailable\t%+v", err) | ||
err = status.WrapWithUnavailable(fmt.Sprintf("DeleteMetaInverse API: Cassandra unavailable:", val.GetVal()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
printf: Sprintf call has arguments but no formatting directives (govet)
@@ -56,24 +57,43 @@ func (s *server) GetMeta(ctx context.Context, key *payload.Meta_Key) (*payload.M | |||
switch { | |||
case errors.IsErrCassandraNotFound(err): | |||
log.Warnf("[GetMeta]\tnot found\t%v\t%s", key.GetKey(), err.Error()) | |||
err = status.WrapWithNotFound(fmt.Sprintf("GetMeta API: not found: key %s", key.GetKey()), err, | |||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
case errors.IsErrCassandraUnavailable(err): | ||
log.Warnf("[GetMeta]\tunavailable\t%+v", err) | ||
err = status.WrapWithUnavailable(fmt.Sprintf("GetMeta API: Cassandra unavailable: key %s", key.GetKey()), err, | ||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
default: | ||
log.Errorf("[GetMeta]\tunknown error\t%+v", err) | ||
err = status.WrapWithUnknown(fmt.Sprintf("GetMeta API: unknown error occurred: key %s", key.GetKey()), err, | ||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
4b4b0d9
to
c8687d1
Compare
@@ -160,10 +216,17 @@ func (s *server) SetMeta(ctx context.Context, kv *payload.Meta_KeyVal) (_ *paylo | |||
err = s.redis.Set(ctx, kv.GetKey(), kv.GetVal()) | |||
if err != nil { | |||
log.Errorf("[SetMeta]\tunknown error\t%+v", err) | |||
err = status.WrapWithInternal(fmt.Sprintf("SetMeta API: failed to store: key %s val %s", kv.GetKey(), kv.GetVal()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 122 characters (lll)
} | ||
log.Errorf("[DeleteMetasInverse]\tunknown error\t%+v", err) | ||
err = status.WrapWithUnknown(fmt.Sprintf("DeleteMetasInverse API: unknown error occurred: vals %#v", vals.GetVals()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 124 characters (lll)
@@ -55,16 +56,30 @@ func (s *server) GetMeta(ctx context.Context, key *payload.Meta_Key) (*payload.M | |||
if err != nil { | |||
if errors.IsErrRedisNotFound(err) { | |||
log.Warnf("[GetMeta]\tnot found\t%v\t%s", key.GetKey(), err.Error()) | |||
err = status.WrapWithNotFound(fmt.Sprintf("GetMeta API: not found: key %s", key.GetKey()), err, | |||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
} | ||
log.Errorf("[GetMeta]\tunknown error\t%+v", err) | ||
err = status.WrapWithUnknown(fmt.Sprintf("GetMeta API: unknown error occurred: key %s", key.GetKey()), err, | ||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
@@ -83,16 +98,30 @@ func (s *server) GetMetas(ctx context.Context, keys *payload.Meta_Keys) (mv *pay | |||
if err != nil { | |||
if errors.IsErrRedisNotFound(err) { | |||
log.Warnf("[GetMetas]\tnot found\t%v\t%s", keys.GetKeys(), err.Error()) | |||
err = status.WrapWithNotFound(fmt.Sprintf("GetMetas API: not found: keys %#v", keys.GetKeys()), err, | |||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
@@ -108,16 +137,29 @@ func (s *server) GetMetaInverse(ctx context.Context, val *payload.Meta_Val) (*pa | |||
if err != nil { | |||
if errors.IsErrRedisNotFound(err) { | |||
log.Warnf("[GetMetaInverse]\tnot found\t%v\t%s", val.GetVal(), err.Error()) | |||
err = status.WrapWithNotFound(fmt.Sprintf("GetMetaInverse API: not found: val %s", val.GetVal()), err, | |||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
ResourceName, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
In the case of Unavailale or Unkonwn errors, it would be better to return the target IP of the failed connection as the resource name in errdetails.ResourceInfo and additionally errdetails.RequestInfo containing the key of the request. |
Agree. I'll revise it. |
Both of gocql and go-redis don't provide informations about requesting Cassandra/Redis hosts. So we cannot embed IPs of these hosts into ResourceInfo. |
I've just noticed that some of the Cassandra errors can be categorized as the gRPC errors other than the Unknown error. So I'll tackle with it. |
Now ready to be reviewed. |
[REBASE] Rebase triggered by rinx for branch: refactor/metas/revise-grpc-status-codes-in-error-cases |
112ef2c
to
57bd288
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Others looks good to me
Updated. |
[REBASE] Rebase triggered by rinx for branch: refactor/metas/revise-grpc-status-codes-in-error-cases |
029b008
to
842ee91
Compare
} | ||
log.Errorf("[GetMetasInverse]\tunknown error\t%+v", err) | ||
err = status.WrapWithUnknown(fmt.Sprintf("GetMetasInverse API: unknown error occurred: vals %#v", vals.GetVals()), err, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
line is 121 characters (lll)
@@ -55,16 +56,34 @@ func (s *server) GetMeta(ctx context.Context, key *payload.Meta_Key) (*payload.M | |||
if err != nil { | |||
if errors.IsErrRedisNotFound(err) { | |||
log.Warnf("[GetMeta]\tnot found\t%v\t%s", key.GetKey(), err.Error()) | |||
err = status.WrapWithNotFound(fmt.Sprintf("GetMeta API: not found: key %s", key.GetKey()), err, | |||
&errdetails.RequestInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
RequestId, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in RequestInfo (exhaustivestruct)
&errdetails.RequestInfo{ | ||
ServingData: errdetails.Serialize(key), | ||
}, | ||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
ResourceName, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
} | ||
log.Errorf("[GetMeta]\tunknown error\t%+v", err) | ||
err = status.WrapWithUnknown(fmt.Sprintf("GetMeta API: unknown error occurred: key %s", key.GetKey()), err, | ||
&errdetails.RequestInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
RequestId, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in RequestInfo (exhaustivestruct)
&errdetails.RequestInfo{ | ||
ServingData: errdetails.Serialize(key), | ||
}, | ||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
ResourceName, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
@@ -83,16 +102,34 @@ func (s *server) GetMetas(ctx context.Context, keys *payload.Meta_Keys) (mv *pay | |||
if err != nil { | |||
if errors.IsErrRedisNotFound(err) { | |||
log.Warnf("[GetMetas]\tnot found\t%v\t%s", keys.GetKeys(), err.Error()) | |||
err = status.WrapWithNotFound(fmt.Sprintf("GetMetas API: not found: keys %#v", keys.GetKeys()), err, | |||
&errdetails.RequestInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
RequestId, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in RequestInfo (exhaustivestruct)
&errdetails.RequestInfo{ | ||
ServingData: errdetails.Serialize(keys), | ||
}, | ||
&errdetails.ResourceInfo{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [golangci] reported by reviewdog 🐶
ResourceName, XXX_NoUnkeyedLiteral, XXX_unrecognized, XXX_sizecache are missing in ResourceInfo (exhaustivestruct)
/rebase |
[REBASE] Rebase triggered by rinx for branch: refactor/metas/revise-grpc-status-codes-in-error-cases |
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
842ee91
to
ea6287b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Rintaro Okamura rintaro.okamura@gmail.com
Description:
Reference:
Related Issue:
Nothing
How Has This Been Tested?:
Nothing
Environment:
Types of changes:
Changes to Core Features:
Checklist: