Skip to content
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

Semantic convention for HTTP start / end times #591

Closed
toumorokoshi opened this issue May 8, 2020 · 12 comments · Fixed by #592
Closed

Semantic convention for HTTP start / end times #591

toumorokoshi opened this issue May 8, 2020 · 12 comments · Fixed by #592
Assignees
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory
Milestone

Comments

@toumorokoshi
Copy link
Member

It doesn't seem that there's a defined semantic convention for when an http instrumentation span should start and end. There's some ambiguity that would be worth clarifying, namely whether these measurements should also consider the duration of additional middleware layers.

As a start, I would argue we should declare that instrumentations should consider the full request/response cycle of a web application (include middlewares).

Reference for discussion origination: open-telemetry/opentelemetry-python#659

This is a more specific discussion of #330.

@bogdandrutu
Copy link
Member

I am not sure if these details should be under "semantic conventions" or other section, but I do agree that they are important to be documented.

@toumorokoshi
Copy link
Member Author

Any suggestions on a new location? otherwise I can just fire a PR.

@toumorokoshi
Copy link
Member Author

Actually, re-thinking through it, "semantic_conventions" may still the be right location, since it's describing what the meanings of specific values should be. I believe it could expand to things like timings, since that ascribes meaning to the number.

@Oberon00
Copy link
Member

Oberon00 commented May 9, 2020

I feel like much of this could be generalized and covered in the docs for span's start/end.

@yurishkuro
Copy link
Member

@Oberon00 +1. Why would we want to call this out just for HTTP spans? It is a general principle - if span represents an operation (be it an RPC or an internal function), its timestamps should be as close to operation start and end as possible.

@toumorokoshi
Copy link
Member Author

I've updated the PR to incorporate an example. re-reading the API spec, I do believe that the existing spec is fairly clear on the measurement:

The Span's start and end timestamps reflect the elapsed real time of the
operation.

But I think the example further clarifies around nuances like:

  • whether data transfer time is considered in the span
  • whether things like http middleware and web framework overhead are considered as part of the timing.

@yurishkuro
Copy link
Member

I would argue we should declare that instrumentations should consider the full request/response cycle of a web application (include middlewares).

I am actually neutral on this as a requirement, I think "it depends". Especially because tracing itself can be implemented as a middleware and the author of that middleware may not have much control over whether it's mounted as the outer-most mw/interceptor, or something in the middle of the MW stack.

@pauldraper
Copy link

pauldraper commented May 21, 2020

How people think about layering OT/OTel has always been confusing to me. These are the events:

tcp.client.start
tcp.server.start
ssl.client.start
ssl.server.start
http.client.start
http.server.start
servlet.start
servlet.end
http.server.end
http.client.end
ssl.server.stop
ssl.client.stop
tcp.server.stop
tcp.client.stop

Personally, I think it makes sense to arrange them as:

http.client:
  http.server:
    servlet:

...and throw on as many layers to that as you want, or ignore them.

But don't try to squish or rearrange them in weird ways.

@toumorokoshi
Copy link
Member Author

I would argue we should declare that instrumentations should consider the full request/response cycle of a web application (include middlewares).

I am actually neutral on this as a requirement, I think "it depends". Especially because tracing itself can be implemented as a middleware and the author of that middleware may not have much control over whether it's mounted as the outer-most mw/interceptor, or something in the middle of the MW stack.

I think it's good to provide some level of guidance or best practice. To your point there are situations where what is desired is not entirely possible (such as control over the middleware layering order), but at the same time there are expectations that if a timing names itself the http method and the route, it will encompass all time spent on that operation.

How people think about layering OT/OTel has always been confusing to me. These are the events:

@pauldraper Sorry, I'm not clear on how to incorporate this into the issue. Can you give an example of how one is squishing or re-arranging the request/response stack you posted, and how we can modify the specification to ensure that doesn't happen?

@pauldraper
Copy link

@toumorokoshi sorry there was some stuff that leaked in from #526 that didn't belong. On topic....

  1. There should be a client span from before starting sending request to after receiving response.
  2. There should be a server span from after starting receiving request to after sending response.
  3. There should be other spans for any internal non-HTTP operations (middleware, main handler, whatever).

@toumorokoshi
Copy link
Member Author

sounds good. I believe this does cover an example in the api/trace.md that addresses your concerns, albeit a little less explicitly:

#592

One challenge I'm having with the PR is the desire to make this a more high-level explanation, but the immediate need to clarify things like what a top level server span should or shouldn't encompass for http.

@pauldraper
Copy link

My opinion is that the client and server HTTP spans should contain just HTTP information: headers, statuses, agents, etc.

Everything else, e.g. invoking a Java servlet handler if the URL matches a known pattern, or running middleware, should be a new child span.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants