Added fields to RpcException message for surfacing in AppInsights #70
Conversation
madelinegordon
requested review from
vrdmr,
gavin-aguiar,
YunchuWang,
surgupta-msft,
satvu,
ejizba,
alrod,
AnatoliB,
kaibocai,
shreyas-gopalakrishna,
amamounelsayed and
Francisco-Gamino
as code owners
|
|
||
| // Worker specifies whether exception is a user exception, | ||
| // for purpose of application insights logging. Defaults to false. | ||
| optional bool is_user_exception = 4; |
There was a problem hiding this comment.
You're the first person to use optional in this file. I assume that's fine? But just surprised it hasn't been used already
There was a problem hiding this comment.
Should be okay, we didn't want to have a breaking change xD
There was a problem hiding this comment.
Yes, maybe because these are the only fields on a message definition that are used in one specific scenario and not in others- we don't want to require that workers explicitly set these fields for exceptions not thrown by user code.
There was a problem hiding this comment.
Hi @madelinegordon @liliankasem I am updating the java worker protofile and it gives me error on the optional as
Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.
I wonder is there anything I missed. Thanks.
There was a problem hiding this comment.
Ah if they're optional by default we should remove the label
Added the following fields for the scenario in which user code throws an exception:
These were added so that the user code exception can be piped through to AI and avoid customer confusion.