diff --git a/Dockerfile b/Dockerfile index 30f686dfd..47ee03d13 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,7 +15,7 @@ # # Base image # -FROM ubuntu:trusty +FROM ubuntu:xenial # # Dependencies @@ -93,21 +93,8 @@ RUN mkdir /tmp/openssl && cd /tmp/openssl && \ # process of gRPC puts the OpenSSL static object files into the gRPC archive # which causes link errors later when the agent is linked with the static # OpenSSL library itself. -RUN git clone --depth=1 --recursive -b v1.21.0 https://github.com/grpc/grpc.git /tmp/grpc && \ +RUN git clone --depth=1 --recursive -b v1.25.0 https://github.com/grpc/grpc.git /tmp/grpc && \ cd /tmp/grpc && \ - -# TODO: Remove patch when GKE Istio versions support HTTP 1.0 or -# gRPC http_cli supports HTTP 1.1 -# This sed command is needed until GKE provides Istio 1.1+ exclusively which -# supports HTTP 1.0. - sed -i 's/1\.0/1.1/g' src/core/lib/http/format_request.cc && \ - -# TODO: Remove patch when GKE Istio versions support unambiguous -# FQDNs in rule sets. -# https://github.com/istio/istio/pull/14405 is merged but wait till GKE -# Istio versions includes this PR - sed -i 's/metadata\.google\.internal\./metadata.google.internal/g' src/core/lib/security/credentials/google_default/google_default_credentials.cc && \ - sed -i 's/metadata\.google\.internal\./metadata.google.internal/g' src/core/lib/security/credentials/credentials.h && \ cd third_party/protobuf && \ ./autogen.sh && \ ./configure --with-pic CXXFLAGS="$(pkg-config --cflags protobuf)" LIBS="$(pkg-config --libs protobuf)" LDFLAGS="-Wl,--no-as-needed" && \ diff --git a/src/throttler_api.cc b/src/throttler_api.cc index c6c311b0c..2eb0f05a8 100644 --- a/src/throttler_api.cc +++ b/src/throttler_api.cc @@ -15,6 +15,7 @@ #include "src/throttler_api.h" #include +#include // NOLINT #include #include "src/clock.h" @@ -248,23 +249,27 @@ bool IsValidServiceName(std::string s) { return true; } -APIThrottler::APIThrottler() - : APIThrottler(DefaultCloudEnv(), DefaultClock(), nullptr) {} +APIThrottler::APIThrottler(JNIEnv* jni) + : APIThrottler(DefaultCloudEnv(), DefaultClock(), nullptr, jni) {} APIThrottler::APIThrottler( CloudEnv* env, Clock* clock, std::unique_ptr - stub) + stub, + JNIEnv* jni) : env_(env), clock_(clock), stub_(std::move(stub)), types_({api::CPU, api::WALL}), + java_version_(JavaVersion(jni)), creation_backoff_envelope_ns_(kBackoffNanos), closed_(false) { grpc_init(); gpr_set_log_function(GRPCLog); + LOG(INFO) << "Java version: " << java_version_; + // Create a random number generator. gen_ = std::default_random_engine(clock_->Now().tv_nsec / 1000); dist_ = std::uniform_int_distribution(0, kRandomRange); @@ -281,6 +286,30 @@ APIThrottler::APIThrottler( } } +std::string APIThrottler::JavaVersion(JNIEnv* jni) { + const std::string kUnknownVersion = "unknown_version"; + + jclass system_class = jni->FindClass("java/lang/System"); + if (system_class == nullptr) { + return kUnknownVersion; + } + jmethodID get_property_method = jni->GetStaticMethodID( + system_class, "getProperty", "(Ljava/lang/String;)Ljava/lang/String;"); + if (get_property_method == nullptr) { + return kUnknownVersion; + } + jstring jstr = reinterpret_cast(jni->CallStaticObjectMethod( + system_class, get_property_method, jni->NewStringUTF("java.version"))); + if (jstr == nullptr) { + return kUnknownVersion; + } + // Copy the returned value and release the memory allocated by JNI. + const char* s = jni->GetStringUTFChars(jstr, nullptr); + std::string ret = std::string(s); + jni->ReleaseStringUTFChars(jstr, s); + return ret; +} + void APIThrottler::SetProfileTypes(const std::vector& types) { types_ = types; } @@ -307,6 +336,14 @@ bool APIThrottler::WaitNext() { profile_.Clear(); ResetClientContext(); + + // The system clock is used here directly, because clock_->now() returns + // CLOCK_MONOTONIC, not CLOCK_REALTIME time. + // The API server sets a 1 hour server-side timeout. All agents should set + // a corresponding 1 hour timeout for CreateProfile requests. + ctx_->set_deadline(std::chrono::system_clock::now() + + std::chrono::hours{1}); + grpc::Status st = stub_->CreateProfile(ctx_.get(), req, &profile_); if (st.ok()) { LOG(INFO) << "Profile created: " << ProfileType() << " " @@ -359,6 +396,13 @@ bool APIThrottler::Upload(std::string profile) { req.mutable_profile()->set_profile_bytes(std::move(profile)); ResetClientContext(); + + // The system clock is used here directly, because clock_->now() returns + // CLOCK_MONOTONIC, not CLOCK_REALTIME time. + // The API server sets a 20 second server-side timeout. All agents should set + // a corresponding 20 second timeout for UpdateProfile requests. + ctx_->set_deadline(std::chrono::system_clock::now() + + std::chrono::seconds{20}); grpc::Status st = stub_->UpdateProfile(ctx_.get(), req, &profile_); if (!st.ok()) { @@ -397,7 +441,8 @@ void APIThrottler::ResetClientContext() { std::lock_guard lock(ctx_mutex_); ctx_.reset(new grpc::ClientContext()); // NOLINT ctx_->AddMetadata("x-goog-api-client", - "gccl/" + std::string(CLOUD_PROFILER_AGENT_VERSION)); + "gccl/" + std::string(CLOUD_PROFILER_AGENT_VERSION) + + " gl-java/" + java_version_); if (closed_) { ctx_->TryCancel(); diff --git a/src/throttler_api.h b/src/throttler_api.h index 71332ae72..2f9618f1d 100644 --- a/src/throttler_api.h +++ b/src/throttler_api.h @@ -17,10 +17,13 @@ #ifndef CLOUD_PROFILER_AGENT_JAVA_THROTTLER_API_H_ #define CLOUD_PROFILER_AGENT_JAVA_THROTTLER_API_H_ +#include + #include #include #include // NOLINT #include +#include #include #include "src/clock.h" @@ -36,12 +39,13 @@ namespace profiler { // Throttler implementation using the Cloud Profiler API. class APIThrottler : public Throttler { public: - APIThrottler(); + explicit APIThrottler(JNIEnv* jni_env); // Testing-only constructor. APIThrottler(CloudEnv* env, Clock* clock, std::unique_ptr - stub); + stub, + JNIEnv* jni_env); // Set the list of supported profile types. The list is used in the profile // creation call to the server to specify the supported types. @@ -56,6 +60,7 @@ class APIThrottler : public Throttler { void Close() override; private: + FRIEND_TEST(APIThrottlerTest, TestCreatesAndUploadsProfile); // Takes a backoff on profile creation error. The backoff duration // may be specified by the server. Otherwise it will be a randomized // exponentially increasing value, bounded by kMaxBackoffNanos. @@ -64,6 +69,9 @@ class APIThrottler : public Throttler { // Resets the client gRPC context for the next call. void ResetClientContext(); + // Determines and returns Java version. + static std::string JavaVersion(JNIEnv* jni_env); + private: CloudEnv* env_; Clock* clock_; @@ -72,6 +80,7 @@ class APIThrottler : public Throttler { stub_; google::devtools::cloudprofiler::v2::Profile profile_; std::vector types_; + const std::string java_version_; // Profile creation error handling. int64_t creation_backoff_envelope_ns_; diff --git a/src/worker.cc b/src/worker.cc index 457c65116..bf6ba64dd 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -47,7 +47,7 @@ void Worker::Start(JNIEnv *jni) { // Initialize the throttler here rather in the constructor, since the // constructor is invoked too early, before the heap profiler is initialized. throttler_ = FLAGS_cprof_profile_filename.empty() - ? std::unique_ptr(new APIThrottler()) + ? std::unique_ptr(new APIThrottler(jni)) : std::unique_ptr( new TimedThrottler(FLAGS_cprof_profile_filename)); diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index 1ec292a77..efb499bde 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -208,8 +208,20 @@ bool HeapMonitor::Supported(jvmtiEnv *jvmti) { // If ever this was run with a JDK before JDK11, it would not set this bit as // it was added at the end of the structure. Therefore this is a cheap way to // check for a runtime "are we running with JDK11+". - return caps.can_generate_sampled_object_alloc_events - && caps.can_generate_garbage_collection_events; + if (!caps.can_generate_sampled_object_alloc_events || + !caps.can_generate_garbage_collection_events) { + // Provide more debug information that this will fail: JVMTI_VERSION and + // sizeof is really lower level but helps figure out compilation + // environments. + LOG(WARNING) << "Capabilites not set up: Sampled: " + << caps.can_generate_sampled_object_alloc_events + << "; GC Collection: " + << caps.can_generate_garbage_collection_events + << "; Size of capabilities: " << sizeof(jvmtiCapabilities) + << "; JVMTI_VERSION: " << JVMTI_VERSION; + return false; + } + return true; #else return false; #endif @@ -226,8 +238,8 @@ void HeapMonitor::AddCallback(jvmtiEventCallbacks *callbacks) { bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval) { #ifdef ENABLE_HEAP_SAMPLING if (!Supported(jvmti)) { - LOG(INFO) << "Heap sampling is not supported by the JVM, disabling the " - << " heap sampling monitor"; + LOG(WARNING) << "Heap sampling is not supported by the JVM, disabling the " + << " heap sampling monitor"; return false; } diff --git a/third_party/perftools/profiles/proto/builder.cc b/third_party/perftools/profiles/proto/builder.cc index a9f5715cc..1e3a29eb9 100644 --- a/third_party/perftools/profiles/proto/builder.cc +++ b/third_party/perftools/profiles/proto/builder.cc @@ -89,7 +89,7 @@ uint64 Builder::FunctionId(const char *name, const char *system_name, return index; } -bool Builder::Emit(string *output) { +bool Builder::Emit(std::string *output) { *output = ""; if (!profile_ || !Finalize()) { return false; @@ -97,7 +97,7 @@ bool Builder::Emit(string *output) { return Marshal(*profile_, output); } -bool Builder::Marshal(const Profile &profile, string *output) { +bool Builder::Marshal(const Profile &profile, std::string *output) { *output = ""; StringOutputStream stream(output); GzipOutputStream gzip_stream(&stream); @@ -119,8 +119,9 @@ bool Builder::MarshalToFile(const Profile &profile, int fd) { } bool Builder::MarshalToFile(const Profile &profile, const char *filename) { - int fd = - TEMP_FAILURE_RETRY(open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0444)); + int fd; + while ((fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0444)) < 0 && + errno == EINTR) {} if (fd == -1) { PLOG(ERROR) << "Failed to open file " << filename; return false; diff --git a/third_party/perftools/profiles/proto/builder.h b/third_party/perftools/profiles/proto/builder.h index 62ae72415..256174cf3 100644 --- a/third_party/perftools/profiles/proto/builder.h +++ b/third_party/perftools/profiles/proto/builder.h @@ -90,12 +90,12 @@ class Builder { // Serializes and compresses the profile into a string, replacing // its contents. It calls Finalize() and returns whether the // encoding was successful. - bool Emit(string *output); + bool Emit(std::string *output); // Serializes and compresses a profile into a string, replacing its // contents. Returns false if there were errors on the serialization // or compression, and the output string will not contain valid data. - static bool Marshal(const Profile &profile, string *output); + static bool Marshal(const Profile &profile, std::string *output); // Serializes and compresses a profile into a file represented by a // file descriptor. Returns false if there were errors on the