Skip to content

Conversation

@dalehamel
Copy link
Member

@dalehamel dalehamel commented Apr 3, 2025

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 run or via ruby API will be stored in two places:

  • meta.vernierUserMetadata
    • easily machine readable
  • meta.extra
    • For the UI to display

How

This moves the metadata accessor for vernier profiles to be namespaced under vernierUserMetadata key. 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_at metadata, 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.

raise ArgumentError unless AVAILABLE_MODES.include?(@mode)

@metadata = params.delete(:metadata)
if Gem.loaded_specs["vernier"].version < Gem::Version.new("1.7.0")
Copy link
Member Author

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
Copy link
Member Author

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)
Copy link
Member Author

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
Copy link
Member Author

@dalehamel dalehamel Apr 3, 2025

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.

@dalehamel dalehamel force-pushed the support-vernier-metadata branch from 22e1e17 to ecb4edd Compare April 4, 2025 14:08
@dalehamel dalehamel marked this pull request as ready for review April 4, 2025 14:15
@dalehamel dalehamel requested a review from bmansoob April 4, 2025 14:15
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")
Copy link
Member Author

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])
Copy link
Member Author

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 } }))
Copy link
Member Author

@dalehamel dalehamel Apr 4, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants