Skip to content

Commit

Permalink
apacheGH-42104: [C++] Fix an OTel test failure and remove needless lo…
Browse files Browse the repository at this point in the history
…gs (apache#42122)

### Rationale for this change

Follow-up to apache#39905, as some issues popped after merging.

### What changes are included in this PR?

- Fixes tests failing due to the `ARROW_LOGGING_BACKEND` env var not being defined
- Removes example log statements accidentally left in PR

### Are these changes tested?

Yes

### Are there any user-facing changes?

No

* GitHub Issue: apache#42104

Authored-by: benibus <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
benibus authored Jun 24, 2024
1 parent 5247870 commit 59267e0
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 10 deletions.
2 changes: 0 additions & 2 deletions cpp/src/arrow/flight/sql/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#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 @@ -64,7 +63,6 @@ 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: 0 additions & 3 deletions cpp/src/arrow/flight/sql/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,11 @@

#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 @@ -578,7 +576,6 @@ 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
2 changes: 0 additions & 2 deletions cpp/src/arrow/flight/transport/grpc/grpc_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
#include "arrow/flight/client_middleware.h"
#include "arrow/flight/cookie_internal.h"
#include "arrow/flight/middleware.h"
#include "arrow/flight/otel_logging_internal.h"
#include "arrow/flight/serialization_internal.h"
#include "arrow/flight/transport.h"
#include "arrow/flight/transport/grpc/serialization_internal.h"
Expand Down Expand Up @@ -929,7 +928,6 @@ class GrpcClientImpl : public internal::ClientTransport {
Status GetFlightInfo(const FlightCallOptions& options,
const FlightDescriptor& descriptor,
std::unique_ptr<FlightInfo>* info) override {
ARROW_FLIGHT_OTELLOG_CLIENT(INFO, "[Example message] func=", __func__);
pb::FlightDescriptor pb_descriptor;
pb::FlightInfo pb_response;

Expand Down
3 changes: 0 additions & 3 deletions cpp/src/arrow/flight/transport/grpc/grpc_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include <grpcpp/grpcpp.h>

#include "arrow/buffer.h"
#include "arrow/flight/otel_logging_internal.h"
#include "arrow/flight/serialization_internal.h"
#include "arrow/flight/server.h"
#include "arrow/flight/server_middleware.h"
Expand All @@ -37,7 +36,6 @@
#include "arrow/flight/transport/grpc/util_internal.h"
#include "arrow/flight/transport_server.h"
#include "arrow/flight/types.h"
#include "arrow/util/logger.h"
#include "arrow/util/logging.h"
#include "arrow/util/uri.h"

Expand Down Expand Up @@ -404,7 +402,6 @@ class GrpcServiceHandler final : public FlightService::Service {
::grpc::Status GetFlightInfo(ServerContext* context,
const pb::FlightDescriptor* request,
pb::FlightInfo* response) {
ARROW_FLIGHT_OTELLOG_SERVER(INFO, "[Example message] func=", __func__);
GrpcServerCallContext flight_context(context);
GRPC_RETURN_NOT_GRPC_OK(
CheckAuth(FlightMethod::GetFlightInfo, context, flight_context));
Expand Down
10 changes: 10 additions & 0 deletions cpp/src/arrow/telemetry/telemetry_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "arrow/telemetry/util_internal.h"
#include "arrow/testing/gtest_util.h"
#include "arrow/testing/util.h"
#include "arrow/util/io_util.h"
#include "arrow/util/logging.h"
#include "arrow/util/tracing_internal.h"

Expand All @@ -35,11 +36,20 @@
namespace arrow {
namespace telemetry {

static constexpr char kLoggingEnvVar[] = "ARROW_LOGGING_BACKEND";

class OtelEnvironment : public ::testing::Environment {
public:
static constexpr std::string_view kLoggerName = "arrow-telemetry-test";

void SetUp() override {
auto maybe_env_var = arrow::internal::GetEnvVar(kLoggingEnvVar);
// The env variable is required to be set to obtain a valid logger, so if it hasn't
// been set in the environment then we do that here.
if (maybe_env_var.status().IsKeyError()) {
ASSERT_OK(arrow::internal::SetEnvVar(kLoggingEnvVar, "arrow_otlp_stderr"));
}

// Implicitly sets up span processors + tracer provider
auto tracer = arrow::internal::tracing::GetTracer();
ARROW_UNUSED(tracer);
Expand Down

0 comments on commit 59267e0

Please sign in to comment.