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

fix javascript server template to include defined metrics in the parameter list for named event record calls #643

Conversation

dmueller
Copy link
Contributor

@dmueller dmueller commented Dec 14, 2023

Pull Request checklist

  • Quality: This PR builds and tests run cleanly
    • make test runs without emitting any warnings
    • make lint runs without emitting any errors
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to language binding APIs are noted explicitly

In the process of understanding how server implementations worked, I was creating my own metrics.yaml and pings.yaml configurations and noticed that the javascript_server output included "identifiers_fxa_account_id" even though that was not defined in my metrics.yaml.

Digging into the template, I realized that the other metrics that I did define were parameters on the methods for the event type metrics I had defined, but they were not passed into the record call.

@dmueller dmueller marked this pull request as ready for review December 14, 2023 17:05
@akkomar akkomar requested a review from badboy December 14, 2023 17:40
@badboy badboy force-pushed the dmueller/javascript-server-fxa-account-id-in-template branch from 976de44 to 665781b Compare December 18, 2023 11:50
@badboy badboy enabled auto-merge (rebase) December 18, 2023 11:50
@badboy badboy disabled auto-merge December 18, 2023 11:50
@badboy badboy enabled auto-merge (squash) December 18, 2023 11:50
@badboy badboy merged commit d02ae92 into mozilla:main Dec 18, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants