Skip to content

Commit

Permalink
bridge: add register string accessor functionality plus expose on And…
Browse files Browse the repository at this point in the history
…roid (#1188)

Description: this PR adds the ability to register string accessors from the platform to the engine. This allows the core layer to access strings from the platform at runtime. Two highlights:

    Eventually this mechanism should be made generic to be able to access and set arbitrary types from the platform. This is very type-specific as a POC. #1192
    This PR only wires up Android's platform layer all the way. A subsequent PR will do the same for iOS.

Risk Level: low - new API
Testing: using the example app.
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
  • Loading branch information
junr03 authored and jpsim committed Nov 29, 2022
1 parent 6ab7cb9 commit d7c8415
Show file tree
Hide file tree
Showing 20 changed files with 187 additions and 6 deletions.
1 change: 1 addition & 0 deletions mobile/examples/kotlin/hello_world/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ kt_android_library(
"AsyncDemoFilter.kt",
"BufferDemoFilter.kt",
"DemoFilter.kt",
"DemoStringAccessor.kt",
"MainActivity.kt",
],
custom_package = "io.envoyproxy.envoymobile.helloenvoykotlin",
Expand Down
10 changes: 10 additions & 0 deletions mobile/examples/kotlin/hello_world/DemoStringAccessor.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package io.envoyproxy.envoymobile.helloenvoykotlin

import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor
import java.nio.ByteBuffer

class DemoStringAccessor : EnvoyStringAccessor {
override fun getString(): ByteBuffer {
return ByteBuffer.wrap("PlatformString".toByteArray(Charsets.UTF_8))
}
}
1 change: 1 addition & 0 deletions mobile/examples/kotlin/hello_world/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class MainActivity : Activity() {
.addFilter { DemoFilter() }
.addFilter { BufferDemoFilter() }
.addFilter { AsyncDemoFilter() }
.addStringAccessor("demo-accessor", DemoStringAccessor())
.setOnEngineRunning { Log.d("MainActivity", "Envoy async internal setup completed") }
.build()

Expand Down
9 changes: 9 additions & 0 deletions mobile/library/common/api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,12 @@ envoy_cc_library(
"@envoy//source/common/common:assert_lib",
],
)

envoy_cc_library(
name = "c_types",
hdrs = ["c_types.h"],
repository = "@envoy",
deps = [
"//library/common/types:c_types_lib",
],
)
25 changes: 25 additions & 0 deletions mobile/library/common/api/c_types.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#pragma once

#include "library/common/types/c_types.h"

// NOLINT(namespace-envoy)

#ifdef __cplusplus
extern "C" { // function pointers
#endif

typedef envoy_data (*envoy_get_string_f)(void* context);

#ifdef __cplusplus
} // function pointers
#endif

/**
* Datatype used to access strings from the platform layer. This accessor is read-only.
*/
// TODO: https://github.com/envoyproxy/envoy-mobile/issues/1192 generalize to access arbitrary
// types.
typedef struct {
envoy_get_string_f get_string;
void* context;
} envoy_string_accessor;
1 change: 1 addition & 0 deletions mobile/library/common/jni/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,6 @@ cc_binary(
deps = [
":jni_utility_lib",
"//library/common:envoy_main_interface_lib",
"//library/common/api:c_types",
],
)
46 changes: 44 additions & 2 deletions mobile/library/common/jni/jni_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <string>

#include "library/common/api/c_types.h"
#include "library/common/extensions/filters/http/platform_bridge/c_types.h"
#include "library/common/jni/jni_utility.h"
#include "library/common/jni/jni_version.h"
Expand Down Expand Up @@ -610,6 +611,24 @@ static const void* jvm_http_filter_init(const void* context) {
return retained_filter;
}

// EnvoyStringAccessor

static envoy_data jvm_get_string(void* context) {
JNIEnv* env = get_env();
jobject j_context = static_cast<jobject>(context);
jclass jcls_JvmStringAccessorContext = env->GetObjectClass(j_context);
jmethodID jmid_getString =
env->GetMethodID(jcls_JvmStringAccessorContext, "getString", "()Ljava/nio/ByteBuffer;");
// Passed as a java.nio.ByteBuffer.
jobject j_data = env->CallObjectMethod(j_context, jmid_getString);
envoy_data native_data = buffer_to_native_data(env, j_data);

env->DeleteLocalRef(jcls_JvmStringAccessorContext);
env->DeleteLocalRef(j_data);

return native_data;
}

// EnvoyHTTPStream

extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_initStream(
Expand Down Expand Up @@ -672,9 +691,9 @@ Java_io_envoyproxy_envoymobile_engine_JniLibrary_registerFilterFactory(JNIEnv* e
api->static_context = retained_context;
api->instance_context = NULL;

register_platform_api(env->GetStringUTFChars(filter_name, nullptr), api);
envoy_status_t result = register_platform_api(env->GetStringUTFChars(filter_name, nullptr), api);
env->DeleteLocalRef(jcls_JvmFilterFactoryContext);
return ENVOY_SUCCESS;
return result;
}

extern "C" JNIEXPORT void JNICALL
Expand Down Expand Up @@ -742,3 +761,26 @@ extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibra

return reset_stream(static_cast<envoy_stream_t>(stream_handle));
}

// EnvoyStringAccessor

extern "C" JNIEXPORT jint JNICALL
Java_io_envoyproxy_envoymobile_engine_JniLibrary_registerStringAccessor(JNIEnv* env, jclass,
jstring accessor_name,
jobject j_context) {

// TODO(goaway): The retained_context leaks, but it's tied to the life of the engine.
// This will need to be updated for https://github.com/lyft/envoy-mobile/issues/332.
jclass jcls_JvmStringAccessorContext = env->GetObjectClass(j_context);
jobject retained_context = env->NewGlobalRef(j_context);

envoy_string_accessor* string_accessor =
(envoy_string_accessor*)safe_malloc(sizeof(envoy_string_accessor));
string_accessor->get_string = jvm_get_string;
string_accessor->context = retained_context;

envoy_status_t result =
register_platform_api(env->GetStringUTFChars(accessor_name, nullptr), string_accessor);
env->DeleteLocalRef(jcls_JvmStringAccessorContext);
return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import android.app.Application;
import io.envoyproxy.envoymobile.engine.types.EnvoyHTTPCallbacks;
import io.envoyproxy.envoymobile.engine.types.EnvoyOnEngineRunning;
import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor;

/* Android-specific implementation of the `EnvoyEngine` interface. */
public class AndroidEngineImpl implements EnvoyEngine {
Expand Down Expand Up @@ -58,4 +59,9 @@ public int recordGaugeAdd(String elements, int amount) {
public int recordGaugeSub(String elements, int amount) {
return envoyEngine.recordGaugeSub(elements, amount);
}

@Override
public int registerStringAccessor(String accessorName, EnvoyStringAccessor accessor) {
return envoyEngine.registerStringAccessor(accessorName, accessor);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ java_library(
"JvmCallbackContext.java",
"JvmFilterContext.java",
"JvmFilterFactoryContext.java",
"JvmStringAccessorContext.java",
],
visibility = ["//visibility:public"],
deps = [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package io.envoyproxy.envoymobile.engine;

import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;
import java.util.regex.Matcher;

import io.envoyproxy.envoymobile.engine.types.EnvoyHTTPFilterFactory;
import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor;

/* Typed configuration that may be used for starting Envoy. */
public class EnvoyConfiguration {
Expand All @@ -18,6 +20,7 @@ public class EnvoyConfiguration {
public final String appVersion;
public final String appId;
public final String virtualClusters;
public final Map<String, EnvoyStringAccessor> stringAccessors;

private static final Pattern UNRESOLVED_KEY_PATTERN = Pattern.compile("\\{\\{ (.+) \\}\\}");

Expand All @@ -34,11 +37,13 @@ public class EnvoyConfiguration {
* @param appVersion the App Version of the App using this Envoy Client.
* @param appId the App ID of the App using this Envoy Client.
* @param virtualClusters the JSON list of virtual cluster configs.
* @param stringAccesssors platform string accessors to register.
*/
public EnvoyConfiguration(String statsDomain, int connectTimeoutSeconds, int dnsRefreshSeconds,
int dnsFailureRefreshSecondsBase, int dnsFailureRefreshSecondsMax,
List<EnvoyHTTPFilterFactory> httpFilterFactories, int statsFlushSeconds,
String appVersion, String appId, String virtualClusters) {
String appVersion, String appId, String virtualClusters,
Map<String, EnvoyStringAccessor> stringAccessors) {
this.statsDomain = statsDomain;
this.connectTimeoutSeconds = connectTimeoutSeconds;
this.dnsRefreshSeconds = dnsRefreshSeconds;
Expand All @@ -49,6 +54,7 @@ public EnvoyConfiguration(String statsDomain, int connectTimeoutSeconds, int dns
this.appVersion = appVersion;
this.appId = appId;
this.virtualClusters = virtualClusters;
this.stringAccessors = stringAccessors;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.envoyproxy.envoymobile.engine.types.EnvoyHTTPCallbacks;
import io.envoyproxy.envoymobile.engine.types.EnvoyOnEngineRunning;
import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor;

/* Wrapper layer for calling into Envoy's C/++ API. */
public interface EnvoyEngine {
Expand Down Expand Up @@ -70,4 +71,6 @@ int runWithConfig(EnvoyConfiguration envoyConfiguration, String logLevel,
* @return A status indicating if the action was successful.
*/
int recordGaugeSub(String elements, int amount);

int registerStringAccessor(String accessor_name, EnvoyStringAccessor accessor);
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package io.envoyproxy.envoymobile.engine;

import java.util.Map;

import io.envoyproxy.envoymobile.engine.types.EnvoyHTTPCallbacks;
import io.envoyproxy.envoymobile.engine.types.EnvoyHTTPFilterFactory;
import io.envoyproxy.envoymobile.engine.types.EnvoyOnEngineRunning;
import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor;

/* Concrete implementation of the `EnvoyEngine` interface. */
public class EnvoyEngineImpl implements EnvoyEngine {
Expand Down Expand Up @@ -69,6 +72,12 @@ public int runWithConfig(EnvoyConfiguration envoyConfiguration, String logLevel,
new JvmFilterFactoryContext(filterFactory));
}

for (Map.Entry<String, EnvoyStringAccessor> entry :
envoyConfiguration.stringAccessors.entrySet()) {
JniLibrary.registerStringAccessor(entry.getKey(),
new JvmStringAccessorContext(entry.getValue()));
}

return runWithConfig(envoyConfiguration.resolveTemplate(JniLibrary.templateString(),
JniLibrary.filterTemplateString()),
logLevel, onEngineRunning);
Expand Down Expand Up @@ -121,4 +130,9 @@ public int recordGaugeAdd(String elements, int amount) {
public int recordGaugeSub(String elements, int amount) {
return JniLibrary.recordGaugeSub(engineHandle, elements, amount);
}

@Override
public int registerStringAccessor(String accessor_name, EnvoyStringAccessor accessor) {
return JniLibrary.registerStringAccessor(accessor_name, new JvmStringAccessorContext(accessor));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,14 @@ protected static native int runEngine(long engine, String config, String logLeve
* filter platform filter configuration.
*/
public static native String filterTemplateString();

/**
* Register a string accessor to get strings from the platform.
*
* @param accessorName, unique name identifying this accessor.
* @param context, context containing logic necessary to invoke the accessor.
* @return int, the resulting status of the operation.
*/
protected static native int registerStringAccessor(String accessorName,
JvmStringAccessorContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package io.envoyproxy.envoymobile.engine;

import java.nio.ByteBuffer;

import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor;

class JvmStringAccessorContext {
private final EnvoyStringAccessor accessor;

public JvmStringAccessorContext(EnvoyStringAccessor accessor) { this.accessor = accessor; }

/**
* Invokes getString callback.
*
* @return ByteBuffer, the string retrieved from the platform.
*/
public ByteBuffer getString() { return accessor.getString(); }
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ java_library(
"EnvoyHTTPFilterFactory.java",
"EnvoyOnEngineRunning.java",
"EnvoyStatus.java",
"EnvoyStringAccessor.java",
],
visibility = ["//visibility:public"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package io.envoyproxy.envoymobile.engine.types;

import java.nio.ByteBuffer;

public interface EnvoyStringAccessor {

/**
* Called to retrieve a string from the Application
*/
ByteBuffer getString();
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class EnvoyConfigurationTest {

@Test
fun `resolving with default configuration resolves with values`() {
val envoyConfiguration = EnvoyConfiguration("stats.foo.com", 123, 234, 345, 456, emptyList(), 567, "v1.2.3", "com.mydomain.myapp", "[test]")
val envoyConfiguration = EnvoyConfiguration("stats.foo.com", 123, 234, 345, 456, emptyList(), 567, "v1.2.3", "com.mydomain.myapp", "[test]", emptyMap())

val resolvedTemplate = envoyConfiguration.resolveTemplate(TEST_CONFIG, FILTER_CONFIG)
assertThat(resolvedTemplate).contains("stats_domain: stats.foo.com")
Expand All @@ -49,7 +49,7 @@ class EnvoyConfigurationTest {

@Test
fun `resolve templates with invalid templates will throw on build`() {
val envoyConfiguration = EnvoyConfiguration("stats.foo.com", 123, 234, 345, 456, emptyList(), 567, "v1.2.3", "com.mydomain.myapp", "[test]")
val envoyConfiguration = EnvoyConfiguration("stats.foo.com", 123, 234, 345, 456, emptyList(), 567, "v1.2.3", "com.mydomain.myapp", "[test]", emptyMap())

try {
envoyConfiguration.resolveTemplate("{{ missing }}", "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.envoyproxy.envoymobile.engine.EnvoyConfiguration
import io.envoyproxy.envoymobile.engine.EnvoyEngine
import io.envoyproxy.envoymobile.engine.EnvoyEngineImpl
import io.envoyproxy.envoymobile.engine.types.EnvoyHTTPFilterFactory
import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor
import java.util.UUID

sealed class BaseConfiguration
Expand Down Expand Up @@ -31,6 +32,7 @@ open class EngineBuilder(
private var appVersion = "unspecified"
private var appId = "unspecified"
private var virtualClusters = "[]"
private var stringAccessors = mutableMapOf<String, EnvoyStringAccessor>()

/**
* Add a log level to use with Envoy.
Expand Down Expand Up @@ -134,6 +136,19 @@ open class EngineBuilder(
return this
}

/**
* Add a string accessor to this Envoy Client.
*
* @param name the name of the accessor.
* @param accessor the string accessor.
*
* @return this builder.
*/
fun addStringAccessor(name: String, accessor: EnvoyStringAccessor): EngineBuilder {
this.stringAccessors.put(name, accessor)
return this
}

/**
* Add the App Version of the App using this Envoy Client.
*
Expand Down Expand Up @@ -186,7 +201,7 @@ open class EngineBuilder(
EnvoyConfiguration(
statsDomain, connectTimeoutSeconds,
dnsRefreshSeconds, dnsFailureRefreshSecondsBase, dnsFailureRefreshSecondsMax,
filterChain, statsFlushSeconds, appVersion, appId, virtualClusters
filterChain, statsFlushSeconds, appVersion, appId, virtualClusters, stringAccessors
),
logLevel, onEngineRunning
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.envoyproxy.envoymobile.engine.EnvoyEngine
import io.envoyproxy.envoymobile.engine.EnvoyHTTPStream
import io.envoyproxy.envoymobile.engine.types.EnvoyHTTPCallbacks
import io.envoyproxy.envoymobile.engine.types.EnvoyOnEngineRunning
import io.envoyproxy.envoymobile.engine.types.EnvoyStringAccessor

/**
* Mock implementation of `EnvoyEngine`. Used internally for testing the bridging layer & mocking.
Expand All @@ -23,4 +24,6 @@ internal class MockEnvoyEngine : EnvoyEngine {
override fun recordGaugeAdd(elements: String, amount: Int): Int = 0

override fun recordGaugeSub(elements: String, amount: Int): Int = 0

override fun registerStringAccessor(accessorName: String, accessor: EnvoyStringAccessor): Int = 0
}
Loading

0 comments on commit d7c8415

Please sign in to comment.