Skip to content

Commit b8d8337

Browse files
duncanpoduncanpo
committed
Fix a segmentation fault with sdk.trace.Cleanup when global TracerProvider not set
1 parent 9706c23 commit b8d8337

File tree

9 files changed

+39
-195
lines changed

9 files changed

+39
-195
lines changed

‎exporters/otlp/+opentelemetry/+exporters/+otlp/OtlpGrpcSpanExporter.m‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,39 +58,39 @@
5858
valuei = optionvalues{i};
5959
if strcmp(namei, "Endpoint")
6060
if ~(isStringScalar(valuei) || (ischar(valuei) && isrow(valuei)))
61-
error("Endpoint must be a scalar string.");
61+
error("opentelemetry:exporters:otlp:OtlpGrpcSpanExporter:EndpointNotScalarText", "Endpoint must be a scalar string.");
6262
end
6363
endpoint = string(valuei);
6464
elseif strcmp(namei, "UseCredentials ")
6565
if ~((islogical(valuei) || isnumeric(valuei)) && isscalar(valuei))
66-
error("UseCredentials must be a scalar logical.")
66+
error("opentelemetry:exporters:otlp:OtlpGrpcSpanExporter:UseCredentialsNotScalarLogical", "UseCredentials must be a scalar logical.")
6767
end
6868
usessl = logical(valuei);
6969
elseif strcmp(namei, "CertificatePath")
7070
if ~(isStringScalar(valuei) || (ischar(valuei) && isrow(valuei)))
71-
error("CertificatePath must be a scalar string.");
71+
error("opentelemetry:exporters:otlp:OtlpGrpcSpanExporter:CertificatePathNotScalarText", "CertificatePath must be a scalar string.");
7272
end
7373
certificatepath = string(valuei);
7474
elseif strcmp(namei, "CertificateString")
7575
if ~(isStringScalar(valuei) || (ischar(valuei) && isrow(valuei)))
76-
error("CertificateString must be a scalar string.");
76+
error("opentelemetry:exporters:otlp:OtlpGrpcSpanExporter:CertificateStringNotScalarText", "CertificateString must be a scalar string.");
7777
end
7878
certificatestring = string(valuei);
7979
elseif strcmp(namei, "Timeout")
8080
if ~(isduration(valuei) && isscalar(valuei))
81-
error("Timeout must be a scalar duration.");
81+
error("opentelemetry:exporters:otlp:OtlpGrpcSpanExporter:TimeoutNotScalarDuration", "Timeout must be a scalar duration.");
8282
end
8383
timeout = valuei;
8484
timeout_millis = milliseconds(timeout);
8585
else % HttpHeaders
8686
if ~isa(valuei, "dictionary")
87-
error("HttpHeaders input must be a dictionary.");
87+
error("opentelemetry:exporters:otlp:OtlpGrpcSpanExporter:HttpHeadersNotDictionary", "HttpHeaders input must be a dictionary.");
8888
end
8989
httpheaders = valuei;
9090
headerkeys = keys(valuei);
9191
headervalues = values(valuei);
9292
if ~isstring(headervalues)
93-
error("HttpHeaders dictionary values must be strings.")
93+
error("opentelemetry:exporters:otlp:OtlpGrpcSpanExporter:HttpHeadersNonStringValues", "HttpHeaders dictionary values must be strings.")
9494
end
9595
end
9696
end

‎exporters/otlp/+opentelemetry/+exporters/+otlp/OtlpHttpSpanExporter.m‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
valuei = optionvalues{i};
5959
if strcmp(namei, "Endpoint")
6060
if ~(isStringScalar(valuei) || (ischar(valuei) && isrow(valuei)))
61-
error("Endpoint must be a scalar string.");
61+
error("opentelemetry:exporters:otlp:OtlpHttpSpanExporter:EndpointNotScalarText", "Endpoint must be a scalar string.");
6262
end
6363
endpoint = string(valuei);
6464
elseif strcmp(namei, "Format")
@@ -67,24 +67,24 @@
6767
jsonbytesmapping = validatestring(valuei, ["hex", "hexId", "base64"]);
6868
elseif strcmp(namei, "UseJsonName")
6969
if ~((islogical(valuei) || isnumeric(valuei)) && isscalar(valuei))
70-
error("UseJsonName must be a scalar logical.")
70+
error("opentelemetry:exporters:otlp:OtlpHttpSpanExporter:UseJsonNameNotScalarLogical", "UseJsonName must be a scalar logical.")
7171
end
7272
usejsonname = logical(valuei);
7373
elseif strcmp(namei, "Timeout")
7474
if ~(isduration(valuei) && isscalar(valuei))
75-
error("Timeout must be a scalar duration.");
75+
error("opentelemetry:exporters:otlp:OtlpHttpSpanExporter:TimeoutNotScalarDuration", "Timeout must be a scalar duration.");
7676
end
7777
timeout = valuei;
7878
timeout_millis = milliseconds(timeout);
7979
else % HttpHeaders
8080
if ~isa(valuei, "dictionary")
81-
error("HttpHeaders input must be a dictionary.");
81+
error("opentelemetry:exporters:otlp:OtlpHttpSpanExporter:HttpHeadersNotDictionary", "HttpHeaders input must be a dictionary.");
8282
end
8383
httpheaders = valuei;
8484
headerkeys = keys(valuei);
8585
headervalues = values(valuei);
8686
if ~isstring(headervalues)
87-
error("HttpHeaders dictionary values must be strings.")
87+
error("opentelemetry:exporters:otlp:OtlpHttpSpanExporter:HttpHeadersNonStringValues", "HttpHeaders dictionary values must be strings.")
8888
end
8989
end
9090
end

‎sdk/trace/+opentelemetry/+sdk/+trace/BatchSpanProcessor.m‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,21 @@
5555
if strcmp(namei, "MaximumQueueSize")
5656
if ~isnumeric(valuei) || ~isscalar(valuei) || valuei <= 0 || ...
5757
round(valuei) ~= valuei
58-
error("opentelemetry:InvalidMaxQueueSize", ...
58+
error("opentelemetry:sdk:trace:BatchSpanProcessor:InvalidMaxQueueSize", ...
5959
"MaximumQueueSize must be a scalar positive integer.");
6060
end
6161
qsize = double(valuei);
6262
elseif strcmp(namei, "ScheduledDelay")
6363
if ~isduration(valuei) || ~isscalar(valuei) || valuei <= 0
64-
error("opentelemetry:InvalidScheduledDelay", ...
64+
error("opentelemetry:sdk:trace:BatchSpanProcessor:InvalidScheduledDelay", ...
6565
"ScheduledDelay must be a positive duration scalar.");
6666
end
6767
delay = valuei;
6868
delaymillis = milliseconds(valuei);
6969
else % "MaximumExportBatchSize"
7070
if ~isnumeric(valuei) || ~isscalar(valuei) || valuei <= 0 || ...
7171
round(valuei) ~= valuei
72-
error("opentelemetry:InvalidMaxExportBatchSize", ...
72+
error("opentelemetry:sdk:trace:BatchSpanProcessor:InvalidMaxExportBatchSize", ...
7373
"MaximumExportBatchSize must be a scalar positive integer.");
7474
end
7575
batchsize = double(valuei);

‎sdk/trace/+opentelemetry/+sdk/+trace/Cleanup.m‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@
1515
% return false if input is not the right type
1616
if isa(tp, "opentelemetry.trace.TracerProvider")
1717
% convert to TracerProvider class in sdk
18-
tpsdk = opentelemetry.sdk.trace.TracerProvider(tp.Proxy);
18+
try
19+
tpsdk = opentelemetry.sdk.trace.TracerProvider(tp.Proxy);
20+
catch
21+
success = false;
22+
return
23+
end
1924
success = tpsdk.shutdown;
2025
postShutdown(tp);
2126
else
@@ -38,7 +43,12 @@
3843
% return false if input is not the right type
3944
if isa(tp, "opentelemetry.trace.TracerProvider")
4045
% convert to TracerProvider class in sdk
41-
tpsdk = opentelemetry.sdk.trace.TracerProvider(tp.Proxy);
46+
try
47+
tpsdk = opentelemetry.sdk.trace.TracerProvider(tp.Proxy);
48+
catch
49+
success = false;
50+
return
51+
end
4252
if nargin < 2 || ~isa(timeout, "duration")
4353
success = tpsdk.forceFlush;
4454
else

‎sdk/trace/+opentelemetry/+sdk/+trace/TracerProvider.m‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,14 @@
7373
valuei = optionvalues{i};
7474
if strcmp(namei, "Sampler")
7575
if ~isa(valuei, "opentelemetry.sdk.trace.Sampler")
76-
error("opentelemetry:InvalidSamplerType", ...
76+
error("opentelemetry:sdk:trace:TracerProvider:InvalidSamplerType", ...
7777
"Sampler must be an instance of one of the sampler classes");
7878
end
7979
sampler = valuei;
8080
foundsampler = true;
8181
else % "Resource"
8282
if ~isa(valuei, "dictionary")
83-
error("opentelemetry:InvalidResourceType", ...
83+
error("opentelemetry:sdk:trace:TracerProvider:InvalidResourceType", ...
8484
"Attibutes input must be a dictionary.");
8585
end
8686
resource = valuei;

‎sdk/trace/src/TracerProviderProxy.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ libmexclass::proxy::MakeResult TracerProviderProxy::make(const libmexclass::prox
2929
libmexclass::proxy::ID tpid = tpid_mda[0];
3030
auto tp = std::static_pointer_cast<libmexclass::opentelemetry::TracerProviderProxy>(
3131
libmexclass::proxy::ProxyManager::getProxy(tpid))->getInstance();
32+
// check if input can be cast to an SDK Tracer Provider
33+
auto tpsdk = dynamic_cast<trace_sdk::TracerProvider*>(tp.get());
34+
if (tpsdk == nullptr) {
35+
return libmexclass::error::Error{"opentelemetry:sdk:trace:Cleanup:UnsetGlobalInstance",
36+
"Clean up operations are not supported if global TracerProvider instance is not set."};
37+
}
3238
out = std::make_shared<TracerProviderProxy>(nostd::shared_ptr<trace_api::TracerProvider>(tp));
3339
} else {
3440
matlab::data::TypedArray<uint64_t> processorid_mda = constructor_arguments[0];

‎test/Performance/traceTest.m‎

Lines changed: 0 additions & 172 deletions
This file was deleted.

‎test/commonTeardown.m‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,6 @@ function commonTeardown(testCase)
33
%
44
% Copyright 2023 The MathWorks, Inc.
55

6-
% Flush all spans that have not yet been exported
7-
tp = opentelemetry.trace.Provider.getTracerProvider();
8-
opentelemetry.sdk.trace.Cleanup.forceFlush(tp);
9-
106
% Terminate Collector if it is still running
117
terminateCollector(testCase);
128

‎test/performance/traceTest.m‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ function setup(testCase)
3636

3737
methods (TestMethodTeardown)
3838
function teardown(testCase)
39+
% Flush all spans that have not yet been exported
40+
tp = opentelemetry.trace.Provider.getTracerProvider();
41+
opentelemetry.sdk.trace.Cleanup.forceFlush(tp);
42+
3943
commonTeardown(testCase);
4044
end
4145
end

0 commit comments

Comments
 (0)