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

Fix for OtlpGrpcExporterOptions in otel-cpp >=1.13.0 #451

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

dpiddock
Copy link
Contributor

@dpiddock dpiddock commented Jun 3, 2024

Fix for building against opentelemetry-cpp 1.13.0 and later, caused by this PR: open-telemetry/opentelemetry-cpp#2388:

[ 88%] Building CXX object CMakeFiles/otel_ngx_module.dir/src/propagate.cpp.o
/opentelemetry-cpp-contrib/instrumentation/nginx/src/otel_ngx_module.cpp: In function 'ngx_int_t OtelGetTraceId(ngx_http_request_t*, ngx_http_variable_value_t*, uintptr_t)':
/opentelemetry-cpp-contrib/instrumentation/nginx/src/otel_ngx_module.cpp:268:81: warning: unused parameter 'data' [-Wunused-parameter]
  268 | OtelGetTraceId(ngx_http_request_t* req, ngx_http_variable_value_t* v, uintptr_t data) {
      |                                                                       ~~~~~~~~~~^~~~
/opentelemetry-cpp-contrib/instrumentation/nginx/src/otel_ngx_module.cpp: In function 'ngx_int_t OtelGetSpanId(ngx_http_request_t*, ngx_http_variable_value_t*, uintptr_t)':
/opentelemetry-cpp-contrib/instrumentation/nginx/src/otel_ngx_module.cpp:323:80: warning: unused parameter 'data' [-Wunused-parameter]
  323 | OtelGetSpanId(ngx_http_request_t* req, ngx_http_variable_value_t* v, uintptr_t data) {
      |                                                                      ~~~~~~~~~~^~~~
/opentelemetry-cpp-contrib/instrumentation/nginx/src/otel_ngx_module.cpp: In function 'std::unique_ptr<opentelemetry::v1::sdk::trace::SpanExporter> CreateExporter(const OtelNgxAgentConfig*)':
/opentelemetry-cpp-contrib/instrumentation/nginx/src/otel_ngx_module.cpp:1024:50: error: no matching function for call to 'opentelemetry::v1::exporter::otlp::OtlpGrpcExporterOptions::OtlpGrpcExporterOptions(<brace-enclosed initializer list>)'
 1024 |       otlp::OtlpGrpcExporterOptions opts{endpoint};
      |                                                  ^     
In file included from /install/include/opentelemetry/exporters/otlp/otlp_grpc_exporter.h:18,
                 from /opentelemetry-cpp-contrib/instrumentation/nginx/src/otel_ngx_module.cpp:5:
/install/include/opentelemetry/exporters/otlp/otlp_grpc_exporter_options.h:26:3: note: candidate: 'opentelemetry::v1::exporter::otlp::OtlpGrpcExporterOptions::OtlpGrpcExporterOptions()'
   26 |   OtlpGrpcExporterOptions();
      |   ^~~~~~~~~~~~~~~~~~~~~~~
/install/include/opentelemetry/exporters/otlp/otlp_grpc_exporter_options.h:26:3: note:   candidate expects 0 arguments, 1 provided
/install/include/opentelemetry/exporters/otlp/otlp_grpc_exporter_options.h:24:29: note: candidate: 'opentelemetry::v1::exporter::otlp::OtlpGrpcExporterOptions::OtlpGrpcExporterOptions(const opentelemetry::v1::exporter::otlp::OtlpGrpcExporterOptions&)'
   24 | struct OPENTELEMETRY_EXPORT OtlpGrpcExporterOptions : public OtlpGrpcClientOptions
      |                             ^~~~~~~~~~~~~~~~~~~~~~~
/install/include/opentelemetry/exporters/otlp/otlp_grpc_exporter_options.h:24:29: note:   no known conversion for argument 1 from 'std::string' {aka 'std::__cxx11::basic_string<char>'} to 'const opentelemetry::v1::exporter::otlp::OtlpGrpcExporterOptions&'
make[2]: *** [CMakeFiles/otel_ngx_module.dir/build.make:132: CMakeFiles/otel_ngx_module.dir/src/otel_ngx_module.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:111: CMakeFiles/otel_ngx_module.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

@dpiddock dpiddock requested a review from a team June 3, 2024 16:03
Copy link

linux-foundation-easycla bot commented Jun 3, 2024

CLA Signed


The committers listed above are authorized under a signed CLA.

@lalitb
Copy link
Member

lalitb commented Jun 3, 2024

@dpiddock thanks for the PR. Changes looks good, but would have to sign the CLA before this can be reviewed/merged.

@marcalff marcalff added instrumentation:nginx Nginx module pr:waiting-on-cla Waiting on CLA labels Jun 3, 2024
@dpiddock
Copy link
Contributor Author

dpiddock commented Jun 3, 2024

@lalitb done.

@marcalff marcalff removed the pr:waiting-on-cla Waiting on CLA label Jun 3, 2024
Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix.

@esigo esigo self-assigned this Jun 3, 2024
Copy link
Member

@esigo esigo left a comment

Choose a reason for hiding this comment

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

LGTM
thanks for the fix :)

@esigo esigo merged commit 58601e5 into open-telemetry:main Jun 8, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants