Skip to content

Commit

Permalink
apacheGH-39898: [C++] Add support for OpenTelemetry logging (apache#3…
Browse files Browse the repository at this point in the history
…9905)

<!--
Thanks for opening a pull request!
If this is your first pull request you can find detailed information on
how
to contribute here:
* [New Contributor's
Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
* [Contributing
Overview](https://arrow.apache.org/docs/dev/developers/overview.html)


If this is not a [minor
PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes).
Could you open an issue for this pull request on GitHub?
https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the
[Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.)
of the Apache Arrow project.

Then could you also rename the pull request title in the following
format?

    GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

    MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

    PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

-->

### Rationale for this change

Supporting OTel logs will help us improve diagnostics/debugging where
OTel tracing is utilized.

### What changes are included in this PR?

Primary changes:
- Bumps `opentelemetry-cpp` version to 1.13.0 to access the stable logs
SDK
- Introduces a new `ARROW_TELEMETRY` module for these additions (and in
anticipation of future OpenMetrics support)
- Integrates Otel logging facilities, provides `telemetry::Logger` class
that wraps an OTel logger and an API for creating loggers via OTel's
global logger provider
- Adds developer-friendly log record exporters, mimicking the
already-existing span exporters

Some auxiliary, but significant additions:
- Adds an extended/re-imagined version of the current `ArrowLog` APIs
that aims to be more flexible and configurable - which the OTel loggers
currently utilize. Notable details:
- Adds an abstract `util::Logger` class that enables creating custom
loggers.
- Adds a `LoggerRegistry` for global access to an arbitrary number of
loggers
- Adds new `ARROW_LOG_*` macros that take individual loggers as a
parameter. Additionally, they can be stripped at compile time based on a
minimum log level
- Definitely looking for opinions on this part of the PR,
specifically...
- Adds `ArrowLogLevel::ARROW_TRACE` as the lowest log level to mirror
the equivalent OTel enums
  
NOTE: I've added some log statements that utilize the OTel facilities to
Flight/FlightSQL - which are driven by the FlightSQL server tests. These
are for demonstrative purposes only and I intend to remove them prior to
merge

### Are these changes tested?

Yes (although the OTel-specific tests are currently light)

### Are there any user-facing changes?

This will introduce new APIs that are likely to be public.

<!--
If there are any breaking changes to public APIs, please uncomment the
line below and explain which changes are breaking.
-->
<!-- **This PR includes breaking changes to public APIs.** -->

<!--
Please uncomment the line below (and provide explanation) if the changes
fix either (a) a security vulnerability, (b) a bug that caused incorrect
or invalid data to be produced, or (c) a bug that causes a crash (even
when the API contract is upheld). We use this to highlight fixes to
issues that may affect users without their knowledge. For this reason,
fixing bugs that cause errors don't count, since those are usually
obvious.
-->
<!-- **This PR contains a "Critical Fix".** -->
* Closes: apache#39898
* GitHub Issue: apache#39898
  • Loading branch information
benibus authored Jun 10, 2024
1 parent 1222089 commit f2057c5
Show file tree
Hide file tree
Showing 27 changed files with 1,410 additions and 10 deletions.
40 changes: 34 additions & 6 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -4613,8 +4613,11 @@ macro(build_opentelemetry)
set(_OPENTELEMETRY_LIBS
common
http_client_curl
logs
ostream_log_record_exporter
ostream_span_exporter
otlp_http_client
otlp_http_log_record_exporter
otlp_http_exporter
otlp_recordable
proto
Expand Down Expand Up @@ -4647,6 +4650,14 @@ macro(build_opentelemetry)
set(_OPENTELEMETRY_STATIC_LIBRARY
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_exporter_otlp_http${CMAKE_STATIC_LIBRARY_SUFFIX}"
)
elseif(_OPENTELEMETRY_LIB STREQUAL "otlp_http_log_record_exporter")
set(_OPENTELEMETRY_STATIC_LIBRARY
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_exporter_otlp_http_log${CMAKE_STATIC_LIBRARY_SUFFIX}"
)
elseif(_OPENTELEMETRY_LIB STREQUAL "ostream_log_record_exporter")
set(_OPENTELEMETRY_STATIC_LIBRARY
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_exporter_ostream_logs${CMAKE_STATIC_LIBRARY_SUFFIX}"
)
else()
set(_OPENTELEMETRY_STATIC_LIBRARY
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_${_OPENTELEMETRY_LIB}${CMAKE_STATIC_LIBRARY_SUFFIX}"
Expand Down Expand Up @@ -4681,9 +4692,16 @@ macro(build_opentelemetry)
IMPORTED_LOCATION)
list(APPEND
OPENTELEMETRY_CMAKE_ARGS
-DWITH_OTLP=ON
-DWITH_OTLP_HTTP=ON
-DWITH_OTLP_GRPC=OFF
# Disabled because it seemed to cause linking errors. May be worth a closer look.
-DWITH_FUNC_TESTS=OFF
# These options are slated for removal in v1.14 and their features are deemed stable
# as of v1.13. However, setting their corresponding ENABLE_* macros in headers seems
# finicky - resulting in build failures or ABI-related runtime errors during HTTP
# client initialization. There may still be a solution, but we disable them for now.
-DWITH_OTLP_HTTP_SSL_PREVIEW=OFF
-DWITH_OTLP_HTTP_SSL_TLS_PREVIEW=OFF
"-DProtobuf_INCLUDE_DIR=${OPENTELEMETRY_PROTOBUF_INCLUDE_DIR}"
"-DProtobuf_LIBRARY=${OPENTELEMETRY_PROTOBUF_INCLUDE_DIR}"
"-DProtobuf_PROTOC_EXECUTABLE=${OPENTELEMETRY_PROTOC_EXECUTABLE}")
Expand Down Expand Up @@ -4757,19 +4775,25 @@ macro(build_opentelemetry)
target_link_libraries(opentelemetry-cpp::resources INTERFACE opentelemetry-cpp::common)
target_link_libraries(opentelemetry-cpp::trace INTERFACE opentelemetry-cpp::common
opentelemetry-cpp::resources)
target_link_libraries(opentelemetry-cpp::logs INTERFACE opentelemetry-cpp::common
opentelemetry-cpp::resources)
target_link_libraries(opentelemetry-cpp::http_client_curl
INTERFACE opentelemetry-cpp::ext CURL::libcurl)
INTERFACE opentelemetry-cpp::common opentelemetry-cpp::ext
CURL::libcurl)
target_link_libraries(opentelemetry-cpp::proto INTERFACE ${ARROW_PROTOBUF_LIBPROTOBUF})
target_link_libraries(opentelemetry-cpp::otlp_recordable
INTERFACE opentelemetry-cpp::trace opentelemetry-cpp::resources
opentelemetry-cpp::proto)
INTERFACE opentelemetry-cpp::logs opentelemetry-cpp::trace
opentelemetry-cpp::resources opentelemetry-cpp::proto)
target_link_libraries(opentelemetry-cpp::otlp_http_client
INTERFACE opentelemetry-cpp::sdk opentelemetry-cpp::proto
INTERFACE opentelemetry-cpp::common opentelemetry-cpp::proto
opentelemetry-cpp::http_client_curl
nlohmann_json::nlohmann_json)
target_link_libraries(opentelemetry-cpp::otlp_http_exporter
INTERFACE opentelemetry-cpp::otlp_recordable
opentelemetry-cpp::otlp_http_client)
target_link_libraries(opentelemetry-cpp::otlp_http_log_record_exporter
INTERFACE opentelemetry-cpp::otlp_recordable
opentelemetry-cpp::otlp_http_client)

foreach(_OPENTELEMETRY_LIB ${_OPENTELEMETRY_LIBS})
add_dependencies(opentelemetry-cpp::${_OPENTELEMETRY_LIB} opentelemetry_ep)
Expand All @@ -4791,7 +4815,11 @@ if(ARROW_WITH_OPENTELEMETRY)
set(opentelemetry-cpp_SOURCE "AUTO")
resolve_dependency(opentelemetry-cpp)
set(ARROW_OPENTELEMETRY_LIBS
opentelemetry-cpp::trace opentelemetry-cpp::ostream_span_exporter
opentelemetry-cpp::trace
opentelemetry-cpp::logs
opentelemetry-cpp::otlp_http_log_record_exporter
opentelemetry-cpp::ostream_log_record_exporter
opentelemetry-cpp::ostream_span_exporter
opentelemetry-cpp::otlp_http_exporter)
get_target_property(OPENTELEMETRY_INCLUDE_DIR opentelemetry-cpp::api
INTERFACE_INCLUDE_DIRECTORIES)
Expand Down
18 changes: 18 additions & 0 deletions cpp/src/arrow/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,7 @@ set(ARROW_UTIL_SRCS
util/int_util.cc
util/io_util.cc
util/list_util.cc
util/logger.cc
util/logging.cc
util/key_value_metadata.cc
util/memory.cc
Expand Down Expand Up @@ -627,6 +628,17 @@ if(ARROW_WITH_ZSTD)
endforeach()
endif()

if(ARROW_WITH_OPENTELEMETRY)
arrow_add_object_library(ARROW_TELEMETRY telemetry/logging.cc)

foreach(ARROW_TELEMETRY_TARGET ${ARROW_TELEMETRY_TARGETS})
target_link_libraries(${ARROW_TELEMETRY_TARGET} PRIVATE ${ARROW_OPENTELEMETRY_LIBS})
endforeach()
else()
set(ARROW_TELEMETRY_TARGET_SHARED)
set(ARROW_TELEMETRY_TARGET_STATIC)
endif()

set(ARROW_TESTING_SHARED_LINK_LIBS arrow_shared ${ARROW_GTEST_GTEST})
set(ARROW_TESTING_SHARED_PRIVATE_LINK_LIBS arrow::flatbuffers RapidJSON)
set(ARROW_TESTING_STATIC_LINK_LIBS arrow::flatbuffers RapidJSON arrow_static
Expand Down Expand Up @@ -1016,6 +1028,7 @@ add_arrow_lib(arrow
${ARROW_JSON_TARGET_SHARED}
${ARROW_MEMORY_POOL_TARGET_SHARED}
${ARROW_ORC_TARGET_SHARED}
${ARROW_TELEMETRY_TARGET_SHARED}
${ARROW_UTIL_TARGET_SHARED}
${ARROW_VENDORED_TARGET_SHARED}
${ARROW_SHARED_PRIVATE_LINK_LIBS}
Expand All @@ -1031,6 +1044,7 @@ add_arrow_lib(arrow
${ARROW_JSON_TARGET_STATIC}
${ARROW_MEMORY_POOL_TARGET_STATIC}
${ARROW_ORC_TARGET_STATIC}
${ARROW_TELEMETRY_TARGET_STATIC}
${ARROW_UTIL_TARGET_STATIC}
${ARROW_VENDORED_TARGET_STATIC}
${ARROW_SYSTEM_LINK_LIBS}
Expand Down Expand Up @@ -1260,6 +1274,10 @@ if(ARROW_SUBSTRAIT)
add_subdirectory(engine)
endif()

if(ARROW_WITH_OPENTELEMETRY)
add_subdirectory(telemetry)
endif()

if(ARROW_TENSORFLOW)
add_subdirectory(adapters/tensorflow)
endif()
4 changes: 4 additions & 0 deletions cpp/src/arrow/flight/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ set(ARROW_FLIGHT_SRCS
transport/grpc/util_internal.cc
types.cc)

if(ARROW_WITH_OPENTELEMETRY)
list(APPEND ARROW_FLIGHT_SRCS otel_logging.cc)
endif()

if(MSVC)
# Protobuf generated files trigger spurious warnings on MSVC.
foreach(GENERATED_SOURCE "${CMAKE_CURRENT_BINARY_DIR}/Flight.pb.cc"
Expand Down
1 change: 1 addition & 0 deletions cpp/src/arrow/flight/flight_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
#ifdef ARROW_WITH_OPENTELEMETRY
#include <opentelemetry/context/propagation/global_propagator.h>
#include <opentelemetry/context/propagation/text_map_propagator.h>
#include <opentelemetry/sdk/trace/processor.h>
#include <opentelemetry/sdk/trace/tracer_provider.h>
#include <opentelemetry/trace/propagation/http_trace_context.h>
#endif
Expand Down
62 changes: 62 additions & 0 deletions cpp/src/arrow/flight/otel_logging.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

#include <string_view>

#include "arrow/flight/otel_logging.h"
#include "arrow/flight/otel_logging_internal.h"
#include "arrow/result.h"
#include "arrow/util/logger.h"
#include "arrow/util/logging.h"

namespace arrow::flight {

namespace {
constexpr std::string_view kGrpcClientName = "arrow-flight-grpc-client-otel";
constexpr std::string_view kGrpcServerName = "arrow-flight-grpc-server-otel";
constexpr std::string_view kSqlClientName = "arrow-flight-sql-client-otel";
constexpr std::string_view kSqlServerName = "arrow-flight-sql-server-otel";
} // namespace

Status RegisterFlightOtelLoggers(const telemetry::OtelLoggingOptions& options) {
for (auto name : {kGrpcClientName, kGrpcServerName, kSqlClientName, kSqlServerName}) {
ARROW_ASSIGN_OR_RAISE(auto logger,
telemetry::OtelLoggerProvider::MakeLogger(name, options));
DCHECK_NE(logger, nullptr);
ARROW_RETURN_NOT_OK(util::LoggerRegistry::RegisterLogger(name, std::move(logger)));
}
return Status::OK();
}

namespace internal {

std::shared_ptr<util::Logger> GetOtelGrpcClientLogger() {
return util::LoggerRegistry::GetLogger(kGrpcClientName);
}
std::shared_ptr<util::Logger> GetOtelGrpcServerLogger() {
return util::LoggerRegistry::GetLogger(kGrpcServerName);
}
std::shared_ptr<util::Logger> GetOtelSqlClientLogger() {
return util::LoggerRegistry::GetLogger(kSqlClientName);
}
std::shared_ptr<util::Logger> GetOtelSqlServerLogger() {
return util::LoggerRegistry::GetLogger(kSqlServerName);
}

} // namespace internal

} // namespace arrow::flight
33 changes: 33 additions & 0 deletions cpp/src/arrow/flight/otel_logging.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

#pragma once

#include "arrow/util/config.h"

#ifdef ARROW_WITH_OPENTELEMETRY
#include "arrow/status.h"
#include "arrow/telemetry/logging.h"
#include "arrow/util/macros.h"

namespace arrow::flight {

ARROW_EXPORT Status
RegisterFlightOtelLoggers(const telemetry::OtelLoggingOptions& options);

} // namespace arrow::flight
#endif
56 changes: 56 additions & 0 deletions cpp/src/arrow/flight/otel_logging_internal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

#pragma once

#include "arrow/util/config.h"

#include "arrow/util/macros.h"
#ifdef ARROW_WITH_OPENTELEMETRY
#include "arrow/flight/otel_logging.h"
#include "arrow/util/logger.h"

namespace arrow::flight::internal {

ARROW_EXPORT std::shared_ptr<util::Logger> GetOtelGrpcClientLogger();
ARROW_EXPORT std::shared_ptr<util::Logger> GetOtelGrpcServerLogger();
ARROW_EXPORT std::shared_ptr<util::Logger> GetOtelSqlClientLogger();
ARROW_EXPORT std::shared_ptr<util::Logger> GetOtelSqlServerLogger();

} // namespace arrow::flight::internal

#define ARROW_FLIGHT_OTELLOG_CLIENT(LEVEL, ...) \
ARROW_LOGGER_CALL(::arrow::flight::internal::GetOtelGrpcClientLogger(), LEVEL, \
__VA_ARGS__)
#define ARROW_FLIGHT_OTELLOG_SERVER(LEVEL, ...) \
ARROW_LOGGER_CALL(::arrow::flight::internal::GetOtelGrpcServerLogger(), LEVEL, \
__VA_ARGS__)
#define ARROW_FLIGHT_OTELLOG_SQL_CLIENT(LEVEL, ...) \
ARROW_LOGGER_CALL(::arrow::flight::internal::GetOtelSqlClientLogger(), LEVEL, \
__VA_ARGS__)
#define ARROW_FLIGHT_OTELLOG_SQL_SERVER(LEVEL, ...) \
ARROW_LOGGER_CALL(::arrow::flight::internal::GetOtelSqlServerLogger(), LEVEL, \
__VA_ARGS__)

#else

#define ARROW_FLIGHT_OTELLOG_CLIENT(LEVEL, ...) ARROW_UNUSED(0)
#define ARROW_FLIGHT_OTELLOG_SERVER(LEVEL, ...) ARROW_UNUSED(0)
#define ARROW_FLIGHT_OTELLOG_SQL_CLIENT(LEVEL, ...) ARROW_UNUSED(0)
#define ARROW_FLIGHT_OTELLOG_SQL_SERVER(LEVEL, ...) ARROW_UNUSED(0)

#endif
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/server_tracing_middleware.cc
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class TracingServerMiddlewareFactory : public ServerMiddlewareFactory {
options.kind = otel::trace::SpanKind::kServer;
options.parent = otel::trace::GetSpan(new_otel_context)->GetContext();

auto* tracer = arrow::internal::tracing::GetTracer();
auto tracer = otel::trace::Provider::GetTracerProvider()->GetTracer("arrow");
auto method_name = ToString(info.method);
auto span = tracer->StartSpan(
method_name,
Expand Down
2 changes: 2 additions & 0 deletions cpp/src/arrow/flight/sql/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <google/protobuf/any.pb.h>

#include "arrow/buffer.h"
#include "arrow/flight/otel_logging_internal.h"
#include "arrow/flight/sql/protocol_internal.h"
#include "arrow/flight/types.h"
#include "arrow/io/memory.h"
Expand Down Expand Up @@ -63,6 +64,7 @@ arrow::Result<FlightDescriptor> GetFlightDescriptorForCommand(
arrow::Result<std::unique_ptr<FlightInfo>> GetFlightInfoForCommand(
FlightSqlClient* client, const FlightCallOptions& options,
const google::protobuf::Message& command) {
ARROW_FLIGHT_OTELLOG_SQL_CLIENT(INFO, "[Example message] func=", __func__);
ARROW_ASSIGN_OR_RAISE(FlightDescriptor descriptor,
GetFlightDescriptorForCommand(command));
return client->GetFlightInfo(options, descriptor);
Expand Down
3 changes: 3 additions & 0 deletions cpp/src/arrow/flight/sql/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@

#include "arrow/buffer.h"
#include "arrow/builder.h"
#include "arrow/flight/otel_logging_internal.h"
#include "arrow/flight/serialization_internal.h"
#include "arrow/flight/sql/protocol_internal.h"
#include "arrow/flight/sql/sql_info_internal.h"
#include "arrow/type.h"
#include "arrow/util/checked_cast.h"
#include "arrow/util/logger.h"

#define PROPERTY_TO_OPTIONAL(COMMAND, PROPERTY) \
COMMAND.has_##PROPERTY() ? std::make_optional(COMMAND.PROPERTY()) : std::nullopt
Expand Down Expand Up @@ -576,6 +578,7 @@ arrow::Result<std::string> CreateStatementQueryTicket(
Status FlightSqlServerBase::GetFlightInfo(const ServerCallContext& context,
const FlightDescriptor& request,
std::unique_ptr<FlightInfo>* info) {
ARROW_FLIGHT_OTELLOG_SQL_SERVER(INFO, "[Example message] func=", __func__);
google::protobuf::Any any;
if (!any.ParseFromArray(request.cmd.data(), static_cast<int>(request.cmd.size()))) {
return Status::Invalid("Unable to parse command");
Expand Down
Loading

0 comments on commit f2057c5

Please sign in to comment.