-
Notifications
You must be signed in to change notification settings - Fork 13
Support vernier metadata #165
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
base: main
Are you sure you want to change the base?
Conversation
| raise ArgumentError unless AVAILABLE_MODES.include?(@mode) | ||
|
|
||
| @metadata = params.delete(:metadata) | ||
| if Gem.loaded_specs["vernier"].version < Gem::Version.new("1.7.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 1.7.0 or greater, we can just pass it directly as as it was added in https://github.com/jhawthorn/vernier/pull/145/files
|
|
||
| return unless vernier_profile | ||
|
|
||
| # Store all vernier metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vernier stores some "internal" metadata that is actually interesting to us and we want to have in the output. We'll copy these to "user metadata"
|
|
||
| # Internal metadata takes precedence over user metadata, but store | ||
| # everything in user metadata | ||
| vernier_profile.meta[:user_metadata]&.merge!(meta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The merge order here matters, so if a user supplied a key that is otherwise internal, the internal one takes precedence since we are jamming both into the same "namespace".
| data = ::Vernier::Output::Firefox.new(vernier_profile).send(:data) | ||
| data[:meta][:mode] = @mode # TODO: https://github.com/jhawthorn/vernier/issues/30 | ||
| data[:meta].merge!(@metadata) if @metadata | ||
| data[:meta][:vernierUserMetadata] ||= meta # for compatibility with < 1.7.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for metadata, we now read directly from this output key. To be compatible with older versions of vernier, we'll store the metadata that was added by the user here if it doesn't exist.
22e1e17 to
ecb4edd
Compare
| gem("activesupport", "~> 5.2") | ||
| gem("railties", "~> 5.2") | ||
| gem("vernier", "~> 1.3.1") if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.2.1") | ||
| gem("vernier", "~> 1.7.0") if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.2.1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bump this for local development of vernier, so that all tests will run.
| assert_equal("wowza", profile.id) | ||
| assert_equal("bar", profile.context) | ||
| assert_equal("spam", profile[:meta][:extrameta]) | ||
| assert_equal("spam", profile.metadata[:extrameta]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was already a bit leaky, we should have been accessing it through the metadata helper method.
|
|
||
| test "#[] forwards to profile metadata" do | ||
| profile = VernierProfile.new(vernier_profile(meta: { interval: 10_000 })) | ||
| profile = VernierProfile.new(vernier_profile(meta: { vernierUserMetadata: { interval: 10_000 } })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are creating a "Mock" profile, these calls to vernier_profile take the vernierUserMedatata key to mimic the structure of a real profile
What
Move's metadata storage into a namespace under "meta" within vernier profiles, and pass "metadata" param directly to the collector on versions >= 1.7.0
Why
Vernier 1.7.0 adds support for metadata as part of the output natively thanks to these two PRs:
jhawthorn/vernier#140
jhawthorn/vernier#145
With these, metadata specified in
vernier runor via ruby API will be stored in two places:How
This moves the metadata accessor for vernier profiles to be namespaced under
vernierUserMetadatakey. For compatibility with older versions of vernier, if this key does not exist metadata is copied here.We also have some "internal" metadata that vernier adds to its "result" object, but does not include in the output json. Since this data is potentially useful, we copy this as if it were user specified. In the case that user specified metadata has the same key, the "internal" one takes precedence. The exception is the
started_atmetadata, which would be useful if it were in realtime, but as it is from a monotonic clock source it is not especially helpful outside of vernier internals.