Skip to content

Implement https://github.com/open-telemetry/opentelemetry-specification/pull/2452 - #1456

Merged
owent merged 93 commits into
open-telemetry:async-changesfrom
owent:async-changes
Jun 17, 2022
Merged

Implement https://github.com/open-telemetry/opentelemetry-specification/pull/2452#1456
owent merged 93 commits into
open-telemetry:async-changesfrom
owent:async-changes

Conversation

@owent

@owent owent commented Jun 16, 2022

Copy link
Copy Markdown
Member

Fixes #1455

Follow the lastest specification changes.

Implement open-telemetry/opentelemetry-specification#2452

I also merged main nefore I start to achive this specification. It bring many changes of main branch.

Please visit https://github.com/open-telemetry/opentelemetry-cpp/pull/1456/files/b4d47e1ffd077c282a6efb86e701d383dcff7b1a..31dfd5a5bfcb17f376d878c3981ad6dbb0c312d8 to view the real changes of this PR.

Changes

  • Update the latest main branch.
  • Remove AsyncBatch*Processor
  • Using configure to decide whether to use async exporting.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

esigo and others added 30 commits March 21, 2022 07:02
* add owent as reviewer

* fix order
Co-authored-by: Tom Tan <Tom.Tan@microsoft.com>
Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com>
Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com>
owent and others added 19 commits May 25, 2022 12:11
…alues (open-telemetry#1427)

* validate histogram value

* handle Nan

* add changelog

* divide by 0 error on windows

* fix markdown lint
Signed-off-by: WenTao Ou <admin@owent.net>

# Conflicts:
#	CHANGELOG.md
Signed-off-by: WenTao Ou <admin@owent.net>
@owent
owent requested a review from a team June 16, 2022 09:58
@codecov

codecov Bot commented Jun 16, 2022

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (async-changes@d7b03e8). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##             async-changes    #1456   +/-   ##
================================================
  Coverage                 ?   84.72%           
================================================
  Files                    ?      155           
  Lines                    ?     4783           
  Branches                 ?        0           
================================================
  Hits                     ?     4052           
  Misses                   ?      731           
  Partials                 ?        0           

Signed-off-by: WenTao Ou <admin@owent.net>
}
# ifdef ENABLE_ASYNC_EXPORT
, asynchronous_mode_ { asynchronous_mode }
# endif

@lalitb lalitb Jun 16, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a thought - Is it necessary to keep the sync mode, or can we have async as default and only option? And ExportResult would always mean that exporter has received the event batch and uploading them. Upload may fail, in which case exporter will retry and eventually log the error.

@owent owent Jun 17, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OTLP may also need always return ExportResult::kSuccess and ignore the result of HTTP client.

Signed-off-by: WenTao Ou <admin@owent.net>
@lalitb

lalitb commented Jun 17, 2022

Copy link
Copy Markdown
Member

Thanks. I have gone through this PR - https://github.com/open-telemetry/opentelemetry-cpp/pull/1456/files/b4d47e1ffd077c282a6efb86e701d383dcff7b1a..31dfd5a5bfcb17f376d878c3981ad6dbb0c312d8 - for relavant changes, and it looks good to me. I think you should be able to merge this to the async-changes branch right?

@owent
owent merged commit daf5091 into open-telemetry:async-changes Jun 17, 2022
@owent
owent deleted the async-changes branch June 17, 2022 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.