Skip to content

Commit

Permalink
Allow setting source tags from Kotlin
Browse files Browse the repository at this point in the history
This adds the FFI boilerplate and changes the DebugActivity
in order to support the new option. Other language bindings,
except for Swift, will be supported using the environment
variables.
  • Loading branch information
Dexterp37 committed Jul 20, 2020
1 parent 074c2ad commit 5782cfb
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 6 deletions.
1 change: 1 addition & 0 deletions docs/user/debugging/android.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ In the above:
| `logPings` | boolean (`--ez`) | If set to `true`, pings are dumped to logcat; defaults to `false` |
| `sendPing` | string (`--es`) | Sends the ping with the given name immediately |
| `debugViewTag` | string (`--es`) | Tags all outgoing pings as debug pings to make them available for real-time validation, on the [Glean Debug View](./debug-ping-view.md). The value must match the pattern `[a-zA-Z0-9-]{1,20}`. **Important**: in older versions of the Glean SDK, this was named `tagPings` |
| `sourceTags` | string array (`--esa`) | Tags outgoing pings with a maximum of 5, comma-separated, tags. The tags must match the pattern `[a-zA-Z0-9-]{1,20}`. The `automation` tag is meant for tagging pings generated on automation: such pings will be special-handled on the pipeline (i.e. discarded from [non-live views](https://docs.telemetry.mozilla.org/cookbooks/bigquery/querying.html#table-layout-and-naming)). Tags starting with `glean` are reserved for future use.|

All [the options](https://developer.android.com/studio/command-line/adb#am) provided to start the activity are passed over to the main activity for the application to process.
This is useful if SDK users wants to debug telemetry while providing additional options to the product to enable specific behaviors.
Expand Down
2 changes: 2 additions & 0 deletions docs/user/debugging/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ Some of the debugging features described above may also be enabled using environ
`true` or `false`. Any other value will be ignored.
- `debugViewTag`: May be set by the `GLEAN_DEBUG_VIEW_TAG` environment variable. Any valid HTTP header value maybe set here
(e.g. any value that matches the regex `[a-zA-Z0-9-]{1,20}`). Invalid values will be ignored.
- `sourgeTags`: May be set by the `GLEAN_SOURCE_TAGS` environment variable. A comma-separated list of valid HTTP header values may be set here
(e.g. any value that matches the regex `[a-zA-Z0-9-]{1,20}`). Invalid values will be ignored.

These variables must be set at runtime, not at compile time. They will be checked upon Glean initialization.

Expand Down
29 changes: 29 additions & 0 deletions glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ open class GleanInternalAPI internal constructor () {
// Keep track of this value before Glean is initialized
private var logPings: Boolean = false

// Keep track of source tags if set before Glean is initialized.
private var sourceTags: Set<String>? = null

// This object holds data related to any persistent information about the metrics ping,
// such as the last time it was sent and the store name
internal lateinit var metricsPingScheduler: MetricsPingScheduler
Expand Down Expand Up @@ -186,6 +189,10 @@ open class GleanInternalAPI internal constructor () {
setLogPings(logPings)
}

// The source tags might have been set before initialize,
// get the cached value and set them.
sourceTags?.let { setSourceTags(it) }

// Get the current value of the dirty flag so we know whether to
// send a dirty startup baseline ping below. Immediately set it to
// `false` so that dirty startup pings won't be sent if Glean
Expand Down Expand Up @@ -634,6 +641,28 @@ open class GleanInternalAPI internal constructor () {
}
}

/**
* Set the source tags to be applied as headers when uploading pings.
*
* If any of the tags is invalid nothing will be set and this function will
* return `false`, although if we are not initialized yet, there won't be any validation.
*
* This is only meant to be used internally by the `GleanDebugActivity`.
*
* @param tags A list of tags, which must be valid HTTP header values.
*/
internal fun setSourceTags(tags: Set<String>): Boolean {
return if (isInitialized()) {
val tagList = StringArray(tags.toList().toTypedArray(), "utf-8")
LibGleanFFI.INSTANCE.glean_set_source_tags(tagList, tags.size).toBoolean()
} else {
sourceTags = tags
// When setting the source tags before initialization,
// we don't validate the tags, thus this function always returns true.
true
}
}

/**
* Set the logPing debug option, when this is `true`
* the payload of assembled ping requests get logged.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ class GleanDebugActivity : Activity() {
* The value must match the pattern `[a-zA-Z0-9-]{1,20}`.
*/
const val TAG_DEBUG_VIEW_EXTRA_KEY = "debugViewTag"
/**
* Tags all outgoing pings as debug pings to make them available for real-time validation.
* The value must match the pattern `[a-zA-Z0-9-]{1,20}`.
*/
const val SOURCE_TAGS_KEY = "sourceTags"
}

// IMPORTANT: These activities are unsecured, and may be triggered by
Expand Down Expand Up @@ -69,7 +74,7 @@ class GleanDebugActivity : Activity() {

// Make sure that at least one of the supported commands was used.
val supportedCommands =
listOf(SEND_PING_EXTRA_KEY, LOG_PINGS_EXTRA_KEY, TAG_DEBUG_VIEW_EXTRA_KEY)
listOf(SEND_PING_EXTRA_KEY, LOG_PINGS_EXTRA_KEY, TAG_DEBUG_VIEW_EXTRA_KEY, SOURCE_TAGS_KEY)

// Enable debugging options and start the application.
intent.extras?.let {
Expand All @@ -93,8 +98,12 @@ class GleanDebugActivity : Activity() {
Glean.setLogPings(logPings)
}

intent.getStringExtra(SEND_PING_EXTRA_KEY)?.let {
Glean.submitPingByName(it)
intent.getStringExtra(SEND_PING_EXTRA_KEY)?.let { name ->
Glean.submitPingByName(name)
}

intent.getStringArrayExtra(SOURCE_TAGS_KEY)?.let { tags ->
Glean.setSourceTags(tags.toSet())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,8 @@ internal interface LibGleanFFI : Library {

fun glean_set_log_pings(value: Byte)

fun glean_set_source_tags(raw_tags: StringArray, raw_tags_count: Int): Byte

// Misc

fun glean_str_free(ptr: Pointer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import mozilla.telemetry.glean.net.HeadersList
import mozilla.telemetry.glean.net.PingUploader
import mozilla.telemetry.glean.net.UploadResult
import mozilla.telemetry.glean.net.HttpResponse
import mozilla.telemetry.glean.private.NoReasonCodes
import mozilla.telemetry.glean.private.PingType
import mozilla.telemetry.glean.testing.GleanTestRule
import org.junit.Rule
import org.robolectric.Shadows.shadowOf
Expand All @@ -38,13 +40,20 @@ import java.util.concurrent.TimeUnit
*/
private class TestPingTagClient(
private val responseUrl: String = Configuration.DEFAULT_TELEMETRY_ENDPOINT,
private val debugHeaderValue: String? = null
private val debugHeaderValue: String? = null,
private val sourceTagsValue: Set<String>? = null
) : PingUploader {
override fun upload(url: String, data: ByteArray, headers: HeadersList): UploadResult {
assertTrue("URL must be redirected for tagged pings",
url.startsWith(responseUrl))
assertEquals("Debug headers must match what the ping tag was set to",
debugHeaderValue, headers.find { it.first == "X-Debug-ID" }!!.second)
debugHeaderValue?.let {
assertEquals("The debug view header must match what the ping tag was set to",
debugHeaderValue, headers.find { it.first == "X-Debug-ID" }!!.second)
}
sourceTagsValue?.let {
assertEquals("The source tags header must match what the ping tag was set to",
sourceTagsValue.joinToString(","), headers.find { it.first == "X-Source-Tags" }!!.second)
}

return HttpResponse(200)
}
Expand Down Expand Up @@ -222,4 +231,37 @@ class GleanDebugActivityTest {
// test assertions will occur
triggerWorkManager(context)
}

@Test
fun `pings are correctly tagged using sourceTags`() {
val testTags = setOf("tag1", "tag2")

// Use the test client in the Glean configuration
val context = ApplicationProvider.getApplicationContext<Context>()
resetGlean(context, Glean.configuration.copy(
httpClient = TestPingTagClient(sourceTagsValue = testTags)
))

// Create a custom ping for testing. Since we're testing headers,
// it's fine for this to be empty.
val customPing = PingType<NoReasonCodes>(
name = "custom",
includeClientId = false,
sendIfEmpty = true,
reasonCodes = listOf()
)

// Set the extra values and start the intent.
val intent = Intent(ApplicationProvider.getApplicationContext<Context>(),
GleanDebugActivity::class.java)
intent.putExtra(GleanDebugActivity.SEND_PING_EXTRA_KEY, "metrics")
intent.putExtra(GleanDebugActivity.SOURCE_TAGS_KEY, testTags.toTypedArray())
launch<GleanDebugActivity>(intent)

customPing.submit()

// This will trigger the call to `fetch()` in the TestPingTagClient which is where the
// test assertions will occur
triggerWorkManager(context)
}
}
2 changes: 2 additions & 0 deletions glean-core/ffi/glean.h
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,8 @@ void glean_set_experiment_inactive(FfiStr experiment_id);

void glean_set_log_pings(uint8_t value);

uint8_t glean_set_source_tags(RawStringArray raw_tags, int32_t tags_count);

void glean_set_upload_enabled(uint8_t flag);

/**
Expand Down
8 changes: 8 additions & 0 deletions glean-core/ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,4 +441,12 @@ pub extern "C" fn glean_set_log_pings(value: u8) {
with_glean_mut(|glean| Ok(glean.set_log_pings(value != 0)));
}

#[no_mangle]
pub extern "C" fn glean_set_source_tags(raw_tags: RawStringArray, tags_count: i32) -> u8 {
with_glean_mut(|glean| {
let tags = from_raw_string_array(raw_tags, tags_count)?;
Ok(glean.set_source_tags(tags))
})
}

define_string_destructor!(glean_str_free);
2 changes: 2 additions & 0 deletions glean-core/ios/Glean/GleanFfi.h
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,8 @@ void glean_set_experiment_inactive(FfiStr experiment_id);

void glean_set_log_pings(uint8_t value);

uint8_t glean_set_source_tags(RawStringArray raw_tags, int32_t tags_count);

void glean_set_upload_enabled(uint8_t flag);

/**
Expand Down

0 comments on commit 5782cfb

Please sign in to comment.