Repository navigation
Conversation
mateenali66
left a comment
There was a problem hiding this comment.
built cda0cd10 (macOS, ABI v1), 1147/1147 tests pass. three things for when this becomes real PRs.
MetricReader::Collectlosing noexcept makes the catch inPeriodicExportingMetricReader::CollectAndExportOnce()reachable, and it returns before advancingforce_flush_notified_sequence_(periodic_exporting_metric_reader.cc:184-204). with an exporter whoseExport()throws,ForceFlush(2s)times out andMeterProvider::Shutdown()with the default timeout never returns.Span::Spanand its attribute lambda stay noexcept (span.cc:62, :81), so a throwingOnStart()terminates insideStartSpan()before the catch at tracer.cc:170. #4700 now catches inside each exporter'sMakeRecordable(), the opposite convention.- part of the clang-tidy drop is the check not seeing through pure virtuals. with
TreatFunctionsWithoutSpecificationAsThrowing: OnlyUndefined(clang-tidy 23) new escapes show atSpan::SpanandLogger::EmitLogRecord.
is TracerProvider::Shutdown() meant to stay an entry point? a throwing exporter Shutdown() now reaches the caller.
mateenali66
left a comment
There was a problem hiding this comment.
716a672 takes noexcept off the two lambdas in StartSpan, but Span::Span is still noexcept (span.h:38, span.cc:62) and calls OnStart() last. at 39fd37d a processor whose OnStart() throws still aborts inside StartSpan(). with only that noexcept removed, the catch at tracer.cc:167 takes it and the call returns the noop span.
the attribute lambda at span.cc:81 is still noexcept too, while Recordable::SetAttribute no longer is.
points 1 and 3 from my last review are unchanged at this head.
DO NOT MERGE
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes