opentracing / specification

A place to document (and discuss) the OpenTracing specification. 🛑 This project is DEPRECATED! https://github.com/opentracing/specification/issues/163
http://opentracing.io/spec
Apache License 2.0
1.17k stars 182 forks source link

Add "Problem Details for HTTP APIs" semantics standard tags #129

Open mciureanu opened 5 years ago

mciureanu commented 5 years ago

Background

The semantics related to errors seem too technical and related on the language.

Problem

In a polyglot microservice environment errors/warning/exceptions/problems could be logged in a custom way by every component. "Errors may be described by OpenTracing in different ways, largely depending on the language.". The "error.kind" and "error.object" don't capture all the useful semantics related to an error. Error investigation in this context looks more like exception logging - which is helpful mainly for developers and not other users that might make use of the instrumentation/tracing. Especially related to the one of the most important errors/problems in our context - errors returned by HTTP services.

Proposal

More microservices are adopting "Problem Details for HTTP APIs" (https://tools.ietf.org/html/rfc7807) in order to communicate problems in a structured way. Adopting as standard tags the fields described in this RFC in case of an error would give the tools to enhance the process related to problem investigation, and give the ability for the tracers to offer more functionalities in their UI.
So something like adding tags like "problem-details.type" "problem-details.title", "problem-details.description", "problem-details.instance", which reflect the problem details RFC.

yurishkuro commented 5 years ago

More microservices are adopting "Problem Details for HTTP APIs"

Is there data to back up this claim? I'm curious because we're internally in a heated discussion about how microservices should model errors (in the gRPC world, but the principle is the same). E.g. gRPC has ErrorDetails protos, but those are passed back via headers, not as the response payload.

mciureanu commented 5 years ago

Not real numbers, but the fact that Microsoft ASP.NET Core team added this standard recently in asp.net core 2.1 brought me to this conclusion: https://blogs.msdn.microsoft.com/webdev/2018/02/27/asp-net-core-2-1-web-apis/

I guess it's tough anyway, since this standard is only about HTTP errors/problems, and can't be used right away for all cases...

austinlparker commented 5 years ago

Given that there's no guarantee that OpenTracing will be used strictly in the context of HTTP APIs, I'm not sure if this is a great fit for mainline spec. I do wonder if it would be something that could extend error.object in a conventional way via a contrib project?

mciureanu commented 5 years ago

The mainline spec contains already stuff specific to HTTP, like http.method, http.status_code, http.url, so why not extend it? It doesn't have to be a complete error specification for all protocols, it could be only about http ones. Maybe it's clearer this way, since there are different standards for errors depending on the protocol.

austinlparker commented 5 years ago

Sure, but it also contains stuff specific to databases, message busses, etc. I get your point though.

yurishkuro commented 5 years ago

I may have missed it in the RFC, but what is the exact list of fields you are proposing to add? We already have protocol-agnostic equivalents to many that you mentioned, eg error.type, error.message, error.object.

mciureanu commented 5 years ago

From the RFC:

3.1. Members of a Problem Details Object

A problem details object can have the following members:

Also, as presented in 3.2. Extension Members, problem type definitions may extend the problem details object with additional members.

Ex: "invalid-params" parameter:

   {
   "type": "https://example.net/validation-error",
   "title": "Your request parameters didn't validate.",
   "invalid-params": [ {
                         "name": "age",
                         "reason": "must be a positive integer"
                       },
                       {
                         "name": "color",
                         "reason": "must be 'green', 'red' or 'blue'"}
                     ]
   }

Existing fields: type = already there: error.kind status = already there: http.status_code detail = already there: error.object

Missing fields:

My plan is to add error.params, and error.title tags, in our problem details middleware that is used in several services, and enhance our custom tracer UI to reflect this. It would have been nice to know if other systems that want to use problem details would do the same thing, and maybe if other tracers would enhance their UI to use these extra semantics, that's why I created this issue/proposal.

austinlparker commented 5 years ago

@mciureanu would you be interested in writing an RFC for this proposal and submitting a PR?