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.

Sign up for GitHub

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

Jump to bottom

Add recommendations about blocking / queuing / resource consumption for language libraries #94

Closed
tigrannajaryan opened this issue Jun 10, 2019 · 4 comments
Closed

Add recommendations about blocking / queuing / resource consumption for language libraries #94

tigrannajaryan opened this issue Jun 10, 2019 · 4 comments
Labels
needs discussion Need more information before all suitable labels can be applied
Milestone
v0.3

Comments

@tigrannajaryan
Copy link
Member

From: #84 (comment)

Some design principles we could explore:

The SDK/exporter should not block the customer's code (except for explicit call to flush and a grace exit), the SDK would rather drop data instead of eating up all the resources.
The SDK/exporter should have a confined resource consumption, e.g. max memory / max network bandwidth.
@tigrannajaryan tigrannajaryan mentioned this issue Jun 10, 2019
Merged
@saiya
Copy link
Contributor

saiya commented Jun 12, 2019

From a user point of view, I strongly agree with the principle The SDK/exporter should not block the customer's code. It is difficult to allow tracing SDK to block application in large / chaotic system (my company is operating such system).

In OpenCensus, I proposed pull request and discussed about the blocking issue (in census-instrumentation/opencensus-java#1837, census-instrumentation/opencensus-specs#262). OpenCensus developer suggested me to wait for OpenTelemetry instead of improving OpenCensus to implement non blocking tracing.

Are there anything I can help/contribute to state the principle explicitly in OpenTelemetry specification documents ( specification/library-guidelines.md)?

@saiya saiya mentioned this issue Jun 12, 2019
Closed
@songy23
Copy link
Member

songy23 commented Jun 12, 2019

Like I mentioned before in census-instrumentation/opencensus-java#1837 (comment), I personally prefer to provide the configuration on blocking/non-blocking through static configs, like manifest/environment variables/flags etc.

Are there anything I can help/contribute to state the principle explicitly in OpenTelemetry specification documents ( specification/library-guidelines.md)?

Speaking of contributions, I think library-guidelines.md is good place to start with since non-blocking will affect the overall behavior of the library (unless others have a better suggestion here).

saiya added a commit to saiya/opentelemetry-specification that referenced this issue Jun 17, 2019
@saiya
Add Performance and Blocking specification
9b24adf 
Performance and Blocking specification is specified in a separate document and
is linked from Language Library Design principles document.

Implements issue: open-telemetry#94
@saiya saiya mentioned this issue Jun 17, 2019
Merged
saiya added a commit to saiya/opentelemetry-specification that referenced this issue Jun 18, 2019
@saiya
PR fix (open-telemetry#94).
53b0519 
- Write about Metrics & Logging to cover entire API
- Write about shut down / flush operations
- Leave room for blocking implementation options (should not block "as default behavior")
- Grammar & syntax fix
saiya added a commit to saiya/opentelemetry-specification that referenced this issue Jun 18, 2019
@saiya
PR fix (open-telemetry#94).
256fa3f 
- Not limit for tracing, metrics.
saiya added a commit to saiya/opentelemetry-specification that referenced this issue Jun 19, 2019
@saiya
PR fix (open-telemetry#94).
cdc70d2 
- Mentioned about inevitable overhead
- Shutdown may block, but it should support configurable timeout also
@SergeyKanzhelev SergeyKanzhelev added this to the API revision: 07-2019 milestone Jun 20, 2019
saiya added a commit to saiya/opentelemetry-specification that referenced this issue Jun 20, 2019
@saiya @songy23
PR fix (open-telemetry#94)
2c8acd1 
- s/traces/telemetry data/
- Syntax fix

Co-Authored-By: Yang Song <[email protected]>
@iredelmeier iredelmeier added the needs discussion Need more information before all suitable labels can be applied label Jul 30, 2019
carlosalberto pushed a commit that referenced this issue Jul 31, 2019
@saiya @carlosalberto
Add Performance and Blocking specification (#130)
63f7804 
* Add Performance and Blocking specification

Performance and Blocking specification is specified in a separate document and
is linked from Language Library Design principles document.

Implements issue: #94

* PR fix (#94).

- Write about Metrics & Logging to cover entire API
- Write about shut down / flush operations
- Leave room for blocking implementation options (should not block "as default behavior")
- Grammar & syntax fix

* PR fix (#94).

- Not limit for tracing, metrics.

* PR fix (#94).

- Mentioned about inevitable overhead
- Shutdown may block, but it should support configurable timeout also

* PR fix (#94)

- s/traces/telemetry data/
- Syntax fix

Co-Authored-By: Yang Song <[email protected]>

* PR fix (#130)

- Remove duplication with #186
- Mention about configurable timeout of flush operation

* PR fix (#130)

- Not specify default strategy (blocking or information loss)
@SergeyKanzhelev SergeyKanzhelev removed this from the API revision: 07-2019 milestone Sep 27, 2019
@bogdandrutu bogdandrutu added this to the Alpha v0.3 milestone Sep 30, 2019
@jmacd
Copy link
Contributor

jmacd commented Jan 22, 2020

@tigrannajaryan Would you say that this can be closed? I believe library-guidelines.md discusses this topic.

@jmacd
Copy link
Contributor

jmacd commented Jan 29, 2020

If this is still needed, please reopen.

@jmacd jmacd closed this as completed Jan 29, 2020
SergeyKanzhelev pushed a commit to SergeyKanzhelev/opentelemetry-specification that referenced this issue Feb 18, 2020
@saiya
Add Performance and Blocking specification (open-telemetry#130)
00715ed 
* Add Performance and Blocking specification

Performance and Blocking specification is specified in a separate document and
is linked from Language Library Design principles document.

Implements issue: open-telemetry#94

* PR fix (open-telemetry#94).

- Write about Metrics & Logging to cover entire API
- Write about shut down / flush operations
- Leave room for blocking implementation options (should not block "as default behavior")
- Grammar & syntax fix

* PR fix (open-telemetry#94).

- Not limit for tracing, metrics.

* PR fix (open-telemetry#94).

- Mentioned about inevitable overhead
- Shutdown may block, but it should support configurable timeout also

* PR fix (open-telemetry#94)

- s/traces/telemetry data/
- Syntax fix

Co-Authored-By: Yang Song <[email protected]>

* PR fix (open-telemetry#130)

- Remove duplication with open-telemetry#186
- Mention about configurable timeout of flush operation

* PR fix (open-telemetry#130)

- Not specify default strategy (blocking or information loss)
TuckTuckFloof pushed a commit to TuckTuckFloof/opentelemetry-specification that referenced this issue Oct 15, 2020
@ankit-bhargava
Enhancements to PR open-telemetry#86 Add minimal tracer example (open…
c86794e 
…-telemetry#94)
rockb1017 pushed a commit to rockb1017/opentelemetry-specification that referenced this issue Nov 18, 2021
@owais
Document recommendations around semantic conventions (open-telemetry#94)
3bf455d 
* Document recommendations around semantic conventions

* Update specification/semantic_conventions.md

Co-authored-by: Mateusz Rzeszutek <[email protected]>

Co-authored-by: Mateusz Rzeszutek <[email protected]>
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 21, 2024
@tigrannajaryan @carlosalberto
Remove temporary advertisement from Logs Vision (open-telemetry#94)
afd5482 
This sentence was added temporarily to encourage participation
in PR discussion. It is no longer relevant since the PR is merged.
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 23, 2024
@tigrannajaryan
Remove temporary advertisement from Logs Vision (open-telemetry#94)
41f6665 
This sentence was added temporarily to encourage participation
in PR discussion. It is no longer relevant since the PR is merged.
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 31, 2024
@saiya @carlosalberto
Add Performance and Blocking specification (open-telemetry#130)
f5518ea 
* Add Performance and Blocking specification

Performance and Blocking specification is specified in a separate document and
is linked from Language Library Design principles document.

Implements issue: open-telemetry#94

* PR fix (open-telemetry#94).

- Write about Metrics & Logging to cover entire API
- Write about shut down / flush operations
- Leave room for blocking implementation options (should not block "as default behavior")
- Grammar & syntax fix

* PR fix (open-telemetry#94).

- Not limit for tracing, metrics.

* PR fix (open-telemetry#94).

- Mentioned about inevitable overhead
- Shutdown may block, but it should support configurable timeout also

* PR fix (open-telemetry#94)

- s/traces/telemetry data/
- Syntax fix

Co-Authored-By: Yang Song <[email protected]>

* PR fix (open-telemetry#130)

- Remove duplication with open-telemetry#186
- Mention about configurable timeout of flush operation

* PR fix (open-telemetry#130)

- Not specify default strategy (blocking or information loss)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs discussion Need more information before all suitable labels can be applied
Projects
None yet
Milestone
v0.3
Development

No branches or pull requests
7 participants
@saiya @bogdandrutu @iredelmeier @jmacd @tigrannajaryan @SergeyKanzhelev @songy23