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

Clarify semantic conventions around span start and end time #592

Conversation

toumorokoshi
Copy link
Member

Fixes #591.

Adding semantic conventions for http start and end times. This has previously caused some ambiguity in the start / end times that instrumentations should target.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

toumorokoshi commented May 10, 2020

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.

I think that's fair. for http in particular web servers (and sometimes clients) incorporate some common middleware that adds to the cost of the request. I felt like the most ambiguity was there.

I could take a stab at clarifying this is a more general place.

Feedback states that the clarification around the meaning of the
span start and end times could be more generic. Starting a proposal
around by adding an example to the API.
@toumorokoshi toumorokoshi changed the title Adding http semantic conventions for span start and end times. Clarify semantic conventions around span start and end time May 10, 2020
specification/trace/api.md Outdated Show resolved Hide resolved
specification/trace/api.md Outdated Show resolved Hide resolved
@carlosalberto
Copy link
Contributor

One thing to consider is to make the Http example a more general RPC one. No strong feeling though. Overall good as an example ;)

Rewording recommendation to use a more generic RPC

Adding a small blurb clarifying sub operations can be instrumented.
@toumorokoshi toumorokoshi force-pushed the feature/span-duration-conventions branch from 1797a7d to 8099078 Compare May 16, 2020 03:52
@toumorokoshi
Copy link
Member Author

One thing to consider is to make the Http example a more general RPC one. No strong feeling though. Overall good as an example ;)

done! rewritten to a more generic RPC.

specification/trace/api.md Outdated Show resolved Hide resolved
specification/trace/api.md Outdated Show resolved Hide resolved
specification/trace/api.md Outdated Show resolved Hide resolved
@bogdandrutu
Copy link
Member

Please resolve comments so we can get this merged :)

toumorokoshi and others added 2 commits May 18, 2020 20:43
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
@toumorokoshi
Copy link
Member Author

@bogdandrutu addressed!

@toumorokoshi
Copy link
Member Author

@bogdandrutu these are addressed now, thanks!

@carlosalberto carlosalberto added spec:trace Related to the specification/trace directory area:api Cross language API specification issue labels Jun 12, 2020
@bogdandrutu bogdandrutu merged commit 4daee3a into open-telemetry:master Jun 30, 2020
codeboten pushed a commit to codeboten/opentelemetry-specification that referenced this pull request Jul 20, 2020
…emetry#592)

* Adding http semantic conventions for span start and end times.

* Corrected send to end, further clarified timings.

* Moving span operation clarification to api

Feedback states that the clarification around the meaning of the
span start and end times could be more generic. Starting a proposal
around by adding an example to the API.

* Addressing feedback

Rewording recommendation to use a more generic RPC

Adding a small blurb clarifying sub operations can be instrumented.

* Apply suggestions from code review

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Christian Neumüller <christian+github@neumueller.me>

* Expanding on child spans

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Semantic convention for HTTP start / end times
6 participants