Extend host_config endpoint with metricsUrl configuration
#9611
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mayagbarnes
force-pushed
the
metrics/host-config
branch
from
bcffb14 to
af9cad7
Compare
mayagbarnes
changed the title
host_config endpoint with metricsUrl configurationhost_config endpoint with metricsUrl configuration
mayagbarnes
added
security-assessment-completed
lukasmasuch
reviewed
Oct 9, 2024
lukasmasuch
reviewed
Oct 10, 2024
mayagbarnes
force-pushed
the
metrics/host-config
branch
from
44f54c5 to
d03fd06
Compare
mayagbarnes
force-pushed
the
metrics-migration
branch
from
c65ee7f to
bac9298
Compare
mayagbarnes
force-pushed
the
metrics/host-config
branch
from
d03fd06 to
fb71c38
Compare
lukasmasuch
approved these changes
Oct 18, 2024
Comment on lines
+96
to
+97
| this.requestDefaultMetricsConfig() | ||
| } |
Collaborator
There was a problem hiding this comment.
nit: We could set actuallySendMetrics to False here in case that this.metricsUrl is undefined after the requestDefaultMetricsConfig to ensure that metrics gathering gets deactivated. But it depends a bit on the full implementation if this is actually required.
mayagbarnes
force-pushed
the
metrics-migration
branch
from
bac9298 to
0890cbf
Compare
mayagbarnes
force-pushed
the
metrics/host-config
branch
from
2a6dd1e to
5737410
Compare
mayagbarnes
added a commit
that referenced
this pull request
Oct 24, 2024
**Metrics Migration Part 1**: This PR extends the `host_config` endpoint
to add a `metricsUrl`, which will serve as the destination URL for
metrics data.
`metricsUrl` for `host_config` endpoint will handle 4 options:
* **`"off"`**: Turns off metrics collection
* **`"<url>"`**: Sends metrics events to the passed URL
* _The logic to build/send event with necessary fields will be added in
a separate PR_
* **`"postMessage"`**: Sends metrics events via `postMessage` API
* _This logic will be added in a separate PR_
* **`" "`**: Default (OS) - telemetry destination url will be fetched
Should no url be received from the BE, the FE will attempt to fetch the
telemetry url from `data.streamlit.io/metrics.json`, caching it in
localStorage if successful. If there is no `metricsUrl`, no telemetry is
sent.
mayagbarnes
added a commit
that referenced
this pull request
Oct 24, 2024
**Metrics Migration Part 3:** As a follow up to the metrics options in hostConfig endpoint (PR #9611), this PR adds the ability to send metrics events to the host via postMessage API using our existing Host <-> Guest communication.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Metrics Migration Part 1: This PR extends the
host_configendpoint to add ametricsUrl, which will serve as the destination URL for metrics data.metricsUrlforhost_configendpoint will handle 4 options:"off": Turns off metrics collection"<url>": Sends metrics events to the passed URL"postMessage": Sends metrics events viapostMessageAPI" ": Default (OS) - telemetry destination url will be fetchedShould no url be received from the BE, the FE will attempt to fetch the telemetry url from
data.streamlit.io/metrics.json, caching it in localStorage if successful. If there is nometricsUrl, no telemetry is sent.Note: Currently using a test Fivetran Webhook
Testing Plan