Use gRPC status code to indicate retryable and not retryable errors #65
Conversation
tigrannajaryan
requested review from
AloisReitbauer,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
reyang,
SergeyKanzhelev,
songy23,
tedsuo and
yurishkuro
as code owners
tigrannajaryan
force-pushed
the
feature/tigran/otlp_statuscodes
branch
from
371bfdf to
bdd91f5
Compare
tigrannajaryan
force-pushed
the
feature/tigran/otlp_statuscodes
branch
from
bdd91f5 to
14ae6d6
Compare
|
@open-telemetry/specs-approvers please have a look. We need this correction to finalize the protocol. |
tigrannajaryan
force-pushed
the
feature/tigran/otlp_statuscodes
branch
from
5ec9304 to
9c41ede
Compare
|
@open-telemetry/specs-approvers please review. |
|
Please review @open-telemetry/specs-approvers |
|
Please review @open-telemetry/specs-approvers |
|
I'm against using grpc codes to encode retryability. You can argue about the retryability of each single error code. I'm fine with adding an grpc code as additional information (though I think a string with an error message would be even better) to boolean |
I agree that arguing about each single error is unfortunate. The problem is that we have to do it regardless. The protocol uses gRPC and you will get a gRPC code in response to the call and you have to make a decision about what to do for each particular code even if we decide to use explicit We can't escape this discussion unless we choose to ignore error handling. |
|
@open-telemetry/specs-approvers Please review. We are not going to have OTLP protocol for Beta unless this has a quick resolution. |
|
Any more thoughts on this? We need to approve or reject this (unclear what to do if rejected) to implement OTLP. |
tigrannajaryan
force-pushed
the
feature/tigran/otlp_statuscodes
branch
from
6b32604 to
3b4f3ba
Compare
|
@tigrannajaryan Looks great! A few, very minor details to fix/update, but otherwise LGTM |
|
@open-telemetry/specs-approvers this is one of the issues that delays implementation of OTLP protocol. Please review. |
OTLP proposal originally used a separate ResultCode enumeration for server to tell the client whether the failed request can be retried or no. After discussion here open-telemetry/opentelemetry-proto#47 (comment) it became clear that the goal can be achieved using gRPC status codes without a need for custom enumeration. This change removes the ResultCode and explains how to use gRPC status codes.
tigrannajaryan
force-pushed
the
feature/tigran/otlp_statuscodes
branch
from
3b4f3ba to
415f5f7
Compare
|
@open-telemetry/specs-approvers all comments are replied / resolved. PTAL and merge if OK. |
|
@open-telemetry/specs-approvers what is the best way to let all language SIGs know about this change? They will need to change OTLP implementation to match this. |
|
@tigrannajaryan That's a general issue that applies to all spec changes and we should introduce a proper process here. Sometimes changes go unnoticed by some SIGs where sometimes this is discovered weeks/months later and most probably a few times not at all. Currently, the safest, while most tedious way would be to manually open an issue in each SIG to which that change applies. |
…pen-telemetry#65) OTLP proposal originally used a separate ResultCode enumeration for server to tell the client whether the failed request can be retried or no. After discussion here open-telemetry/opentelemetry-proto#47 (comment) it became clear that the goal can be achieved using gRPC status codes without a need for custom enumeration. This change removes the ResultCode and explains how to use gRPC status codes. Co-authored-by: Yuri Shkuro <[email protected]>
…pen-telemetry#65) OTLP proposal originally used a separate ResultCode enumeration for server to tell the client whether the failed request can be retried or no. After discussion here open-telemetry/opentelemetry-proto#47 (comment) it became clear that the goal can be achieved using gRPC status codes without a need for custom enumeration. This change removes the ResultCode and explains how to use gRPC status codes. Co-authored-by: Yuri Shkuro <[email protected]>
…pen-telemetry#65) OTLP proposal originally used a separate ResultCode enumeration for server to tell the client whether the failed request can be retried or no. After discussion here open-telemetry/opentelemetry-proto#47 (comment) it became clear that the goal can be achieved using gRPC status codes without a need for custom enumeration. This change removes the ResultCode and explains how to use gRPC status codes. Co-authored-by: Yuri Shkuro <[email protected]>
…pen-telemetry/oteps#65) OTLP proposal originally used a separate ResultCode enumeration for server to tell the client whether the failed request can be retried or no. After discussion here open-telemetry/opentelemetry-proto#47 (comment) it became clear that the goal can be achieved using gRPC status codes without a need for custom enumeration. This change removes the ResultCode and explains how to use gRPC status codes. Co-authored-by: Yuri Shkuro <[email protected]>
OTLP proposal originally used a separate ResultCode enumeration for server
to tell the client whether the failed request can be retried or no.
After discussion here open-telemetry/opentelemetry-proto#47 (comment)
it became clear that the goal can be achieved using gRPC status codes without
a need for custom enumeration.
This change removes the ResultCode and explains how to use gRPC status codes.