Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1648140: Python: Make sure core metrics are in every ping #1012

Merged
merged 3 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

[Full changelog](https://github.com/mozilla/glean/compare/v31.2.0...main)

* Python
* BUGFIX: Core metrics are now present in every ping, even if submit is called before initialize has a chance to complete. ([#1012](https://github.com/mozilla/glean/pull/1012))

# v31.2.0 (2020-06-24)

[Full changelog](https://github.com/mozilla/glean/compare/v31.1.2...v31.2.0)
Expand Down
16 changes: 9 additions & 7 deletions glean-core/python/glean/glean.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,18 +417,20 @@ def _initialize_core_metrics(cls) -> None:
"""
from ._builtins import metrics

metrics.glean.baseline.locale.set(_util.get_locale_tag())
metrics.glean.internal.metrics.os_version.set(platform.release())
metrics.glean.internal.metrics.architecture.set(platform.machine())
metrics.glean.internal.metrics.locale.set(_util.get_locale_tag())
metrics.glean.baseline.locale._set_sync(_util.get_locale_tag())
metrics.glean.internal.metrics.os_version._set_sync(platform.release())
metrics.glean.internal.metrics.architecture._set_sync(platform.machine())
metrics.glean.internal.metrics.locale._set_sync(_util.get_locale_tag())

if cls._configuration.channel is not None:
metrics.glean.internal.metrics.app_channel.set(cls._configuration.channel)
metrics.glean.internal.metrics.app_channel._set_sync(
cls._configuration.channel
)

metrics.glean.internal.metrics.app_build.set(cls._application_id)
metrics.glean.internal.metrics.app_build._set_sync(cls._application_id)

if cls._application_version is not None:
metrics.glean.internal.metrics.app_display_version.set(
metrics.glean.internal.metrics.app_display_version._set_sync(
cls._application_version
)

Expand Down
13 changes: 13 additions & 0 deletions glean-core/python/glean/metrics/string.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,19 @@ def set(self, value: str) -> None:
def set():
_ffi.lib.glean_string_set(self._handle, _ffi.ffi_encode_string(value))

def _set_sync(self, value: str) -> None:
"""
Set a string value, synchronously.

Args:
value (str): This is a user-defined string value. If the length of
the string exceeds the maximum length, it will be truncated.
"""
if self._disabled:
return

_ffi.lib.glean_string_set(self._handle, _ffi.ffi_encode_string(value))

def test_has_value(self, ping_name: Optional[str] = None) -> bool:
"""
Tests whether a value is stored for the metric for testing purposes
Expand Down
48 changes: 48 additions & 0 deletions glean-core/python/tests/test_glean.py
Original file line number Diff line number Diff line change
Expand Up @@ -682,3 +682,51 @@ def test_confirm_enums_match_values_in_glean_parser():
]:
for name in gp_enum.__members__.keys():
assert g_enum[name.upper()].value == gp_enum[name].value


def test_presubmit_makes_a_valid_ping(tmpdir, ping_schema_url, monkeypatch):
# Bug 1648140: Submitting a ping prior to initialize meant that the core
# metrics wouldn't yet be set.

info_path = Path(str(tmpdir)) / "info.txt"

Glean._reset()

ping_name = "preinit_ping"
ping = PingType(
name=ping_name, include_client_id=True, send_if_empty=True, reason_codes=[]
)

# This test relies on testing mode to be disabled, since we need to prove the
# real-world async behaviour of this.
Dispatcher._testing_mode = False
Dispatcher._queue_initial_tasks = True

# Submit a ping prior to calling initialize
ping.submit()
Glean.initialize(
application_id=GLEAN_APP_ID,
application_version=glean_version,
upload_enabled=True,
configuration=Glean._configuration,
)

monkeypatch.setattr(
Glean._configuration, "ping_uploader", _RecordingUploader(info_path)
)

# Wait until the work is complete
Dispatcher._task_worker._queue.join()

while not info_path.exists():
time.sleep(0.1)

with info_path.open("r") as fd:
url_path = fd.readline()
serialized_ping = fd.readline()

assert ping_name == url_path.split("/")[3]

assert 0 == validate_ping.validate_ping(
io.StringIO(serialized_ping), sys.stdout, schema_url=ping_schema_url,
)