-
Notifications
You must be signed in to change notification settings - Fork 124
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
Include operation_id in error strings #150
Comments
Just reviving this a bit -- we've seen this a few times recently. I think this is a good idea. cc @jeefy (@ethernetdan is no longer on osde2e). |
jhernand
added a commit
to jhernand/ocm-api-metamodel
that referenced
this issue
Feb 26, 2020
This patch changes the code generator so that errors support the `operation_id` attribute. This attribute and the existing ones will now be used to generate more complete error messages. For example, the following JSON error response: ``` { "kind": "Error", "id": "401", "href": "/api/clusters_mgmt/v1/errors/401", "code": "CLUSTERS-MGMT-401", "reason": "My reason", "operation_id": "456" } ``` Will result in the following error string (in one single line): ``` identifier is '401', code is 'CLUSTERS-MGMT-401' and operation identifier is '456': My reason ``` Related: openshift-online/ocm-sdk-go#150 Signed-off-by: Juan Hernandez <jhernand@redhat.com>
jhernand
added a commit
to jhernand/ocm-api-metamodel
that referenced
this issue
Feb 26, 2020
This patch changes the code generator so that errors support the `operation_id` attribute. This attribute and the existing ones will now be used to generate more complete error messages. For example, the following JSON error response: ``` { "kind": "Error", "id": "401", "href": "/api/clusters_mgmt/v1/errors/401", "code": "CLUSTERS-MGMT-401", "reason": "My reason", "operation_id": "456" } ``` Will result in the following error string (in one single line): ``` identifier is '401', code is 'CLUSTERS-MGMT-401' and operation identifier is '456': My reason ``` Related: openshift-online/ocm-sdk-go#150 Signed-off-by: Juan Hernandez <jhernand@redhat.com>
jhernand
added a commit
to jhernand/ocm-api-metamodel
that referenced
this issue
Feb 26, 2020
This patch changes the code generator so that errors support the `operation_id` attribute. This attribute and the existing ones will now be used to generate more complete error messages. For example, the following JSON error response: ``` { "kind": "Error", "id": "401", "href": "/api/clusters_mgmt/v1/errors/401", "code": "CLUSTERS-MGMT-401", "reason": "My reason", "operation_id": "456" } ``` Will result in the following error string (in one single line): ``` identifier is '401', code is 'CLUSTERS-MGMT-401' and operation identifier is '456': My reason ``` Related: openshift-online/ocm-sdk-go#150 Signed-off-by: Juan Hernandez <jhernand@redhat.com>
jhernand
added a commit
to jhernand/ocm-api-metamodel
that referenced
this issue
Feb 26, 2020
The more relevant changes in the new version are the following: - Add `operation_id` attribute to error objects and error messages. Related: openshift-online/ocm-sdk-go#150 Signed-off-by: Juan Hernandez <jhernand@redhat.com>
jhernand
added a commit
to jhernand/ocm-sdk-go
that referenced
this issue
Feb 26, 2020
The more relevant changes in the new version of the metamodel are the following: - Add `operation_id` attribute to error objects and error messages. Related: openshift-online#150 Signed-off-by: Juan Hernandez <jhernand@redhat.com>
Merged
jhernand
added a commit
to jhernand/ocm-sdk-go
that referenced
this issue
Feb 26, 2020
The more relevant changes in the new version are the following: - Update to metamodel 0.0.26: ** Add `operation_id` attribute to error objects and error messages. Related: openshift-online#150 Signed-off-by: Juan Hernandez <jhernand@redhat.com>
This is fixed in release 0.1.89. For a JSON error document like this: {
"kind": "Error",
"id": "401",
"href": "/api/clusters_mgmt/v1/errors/401",
"code": "CLUSTERS-MGMT-401",
"reason": "My reason",
"operation_id": "456"
} The
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Here's an example of a pretty idiomatic SDK usage:
https://github.com/openshift/osde2e/blob/143f353ea48119e9dc3e65868cc2466e738c9943/pkg/osd/cluster.go#L74-L103
(where
u.clusters()
returnsu.conn.ClustersMgmt().V1().Clusters()
)This code eventually logged the following error:
where most of the error is from the calling osde2e code, apparently
CLUSTERS-MGMT-500
is the only part that came from the SDK.In this case, I failed to find matching clusters-service logs to this 500 (interal hive-integration env).
Almost always 500 errors include an
"operation_id": "...."
field, and these make later searching much easier.I think it would be good if the SDK included such id, when available, in the error string it returns.
I guess the reason we didn't do this already is our internal code using the SDK tends to log the full response body on errors. In osde2e code, the immediate function returns the body too:
but then the calling function ignores the body because
err != nil
. Which I think is likely to happen in real-world usage, frequently only the error string propogates out...@jhernand what do you think? cc @meowfaceman @ethernetdan from osde2e
The text was updated successfully, but these errors were encountered: