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

Rewrite NotificationsTrace #5270

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Rewrite NotificationsTrace #5270

wants to merge 7 commits into from

Conversation

rmosolgo
Copy link
Owner

@rmosolgo rmosolgo commented Mar 10, 2025

Trying to update this to handle fiber starts/stops better.

  • Do I really need begin_/finish_ methods for parts of execution that don't call out to app code? (no)
  • Update NotificationsTrace
  • Add a test for AS::N trace
  • Migrate other traces to this system - Started with New Relic and Sentry.
  • What about AS::N payloads? (And Dry::Monitor) I want avoid creating that Hash by default, but those will need it for backwards compat.

end

def instrument(keyword, payload, &block)
raise "Implement #{self.class}#instrument to measure the block"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method instrument missing tests for line 19 (coverage: 0.0)

@platform_resolve_type_key_cache[payload]
when :dataloader_source
@platform_source_class_key_cache[payload.class]
when :parse then self.class::PARSE_NAME

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method name_for missing tests for lines 56, 57, 62, 56, 57, 61 (coverage: 0.75)

attr_reader :keyword, :payload

def start
raise "Implement #{self.class}#start to begin a new event (#{inspect})"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method start missing tests for line 76 (coverage: 0.0)

end

def finish
raise "Implement #{self.class}#finish to end this event (#{inspect})"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method finish missing tests for line 80 (coverage: 0.0)

}.compare_by_identity

def instrument(keyword, payload, &block)
name = EVENT_NAMES.fetch(keyword)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method instrument missing tests for lines 99, 100 (coverage: 0.0)


class Event < NotificationsTrace::Engine::Event
def start
@name = EVENT_NAMES.fetch(@keyword)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method start missing tests for lines 105, 106 (coverage: 0.0)

RUBY

def finish
Dry::Monitor.stop(@name, @payload)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method finish missing tests for line 110 (coverage: 0.0)

# @param trace_resolve_type [Boolean] If `false`, skip tracing `resolve_type?` calls
def initialize(engine:, set_transaction_name: false, trace_scalars: false, trace_authorized: true, trace_resolve_type: true, **rest)
if defined?(Dry::Monitor) && engine == Dry::Monitor
# Backwards compat

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method initialize missing tests for lines 123, 122 (coverage: 0.75)


def lex(query_string:)
@notifications_engine.instrument(:lex, query_string) do
super

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block block missing tests for line 141 (coverage: 0.0)

operation_names = queries.map{|q| operation_name(q) }
span.set_description(operation_names.join(", "))

if queries.size == 1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block block (span) missing tests for lines 33, 33, 33 (coverage: 0.9444)

selected_op = query.selected_operation
if selected_op
[selected_op.operation_type, selected_op.name].compact.join(' ')
else

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method operation_name missing tests for lines 66, 65 (coverage: 0.6)

def instrument_sentry_execution(platform_key, trace_method, data=nil, &block)
return yield unless Sentry.initialized?
def platform_resolve_type_key(type)
"graphql.resolve_type.#{type.graphql_name}"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method platform_resolve_type_key missing tests for line 79 (coverage: 0.0)

result = yield
return result unless span
def platform_source_class_key(source_class)
"graphql.source.#{source_class.name.gsub("::", ".")}"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method platform_source_class_key missing tests for line 83 (coverage: 0.0)

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.

1 participant