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

Supportability enabled/disabled metrics #2286

Merged
merged 502 commits into from
Oct 27, 2023
Merged

Conversation

hannahramadan
Copy link
Contributor

@hannahramadan hannahramadan commented Oct 24, 2023

Add security agent enabled/disabled supportability metric

closes #2063

fallwith and others added 30 commits August 29, 2023 11:52
Sidekiq arg capturing entry wording fix

Co-authored-by: Kayla Reopelle (she/her) <[email protected]>
Sidekiq arg capturing entry: specify YAML as the syntax to use for the newrelic.yml code block

Co-authored-by: Kayla Reopelle (she/her) <[email protected]>
Update the CHANGELOG entry for Sidekiq args filtration to more clearly
explain regexp, link to RubyDocs for regexp, and explain inexact
matches.
Sidekiq args filtering - in `default_source.rb`, use `dynamic_name:
true`, and use heredocs for the descriptions.

By using `dynamic_name: true`, we don't have to have an ignore list for
config scanning.
remove junk that was accidentally added
The agent uses a non-traditional way to subscribe to Rails events.
Instead of passing a block to ActiveSupport::Notifications.subscribe, the
agent defines a #start and #finish method, which Rails responds to.
This is due to a threading issue discovered on initial instrumentation.
Issue: rails/rails#12069
Rails code: https://github.com/rails/rails/blob/ed5af004598fa7645fec2210453cca00f4b59168/activesupport/lib/active_support/notifications/fanout.rb#L320
Given that the existing attributing processing methods and the new
attribute pre-filtering methods don't share any content or logic, have
the pre-filtering methods live in a separate dedicated
`AttributePreFiltering` class.
added source code comments for segment callback functionality
SIdekiq args filtering entry: "has" -> "hash" typo fix

Co-authored-by: Kayla Reopelle (she/her) <[email protected]>
sidekiq.args.include description text updates

Co-authored-by: Kayla Reopelle (she/her) <[email protected]>
sidekiq.args.exclude description text updates

Co-authored-by: Kayla Reopelle (she/her) <[email protected]>
Dependency detection, subscriber, config
the "without exclude" test should actually operate without an exclude
list. thanks, @kaylareopelle
supportability metric names can't be dynamic. use
`:set_segment_callback` and add a test
Previously, the `port_path_or_id` value for Elasticsearch segments
was set to the path given to the `request_with_tracing` method. This
caused a metrics grouping issue for a customer because a new
instance metric ("Datastore/instance/Elasticsearch/<host>/<port>")
was created for every Elasticsearch document ID.

The source of the host value is a hash that also contains a port key. This
commit updates the value of `port_path_or_id` to use the port from the
`nr_hosts` hash.
Our CI system recently errored out on resolving dependency issues with
the `json` gem while prepping for the Sidekiq multiverse suite. Given
the recent reworking of the Sidekiq suite, we should no longer need
anything but Sidekiq and the agent.
- Use `@@` to have the CLI instance memoization actually work as desired
  now that it lives in a helpers module shared by 2 test classes
- Proactively guard against the CLI instance having a `nil` config
only apply the Sidekiq CLI config fix to Sidekiq v7+
GHA is having issues that cannot be reproduced elsewhere
CI: Sidekiq multiverse - remove old dependencies
- Update the deprecation warning text for Sidekiq v5
- Update the CI testing of Sidekiq v5 to stop testing it with Rubies
  newer than 2.5
fallwith
fallwith previously approved these changes Oct 24, 2023
Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

I think some of the test failures are related to this PR. This one is coming from Ruby 3.2.2 and rails61.

NewRelic::Control::SecurityInterfaceTest#test_initialization_short_circuits_if_the_agent_has_already_been_started [/home/runner/work/newrelic-ruby-agent/newrelic-ruby-agent/test/new_relic/control/security_interface_test.rb:74]:
Did not find stats for spec #<NewRelic::MetricSpec 'Supportability/Ruby/SecurityAgent/Agent/Enabled/enabled':''>.
All specs in there were: [
  Supportability/API/drop_buffered_data (<unscoped>),
  Supportability/API/manual_start (<unscoped>),
  Supportability/API/shutdown (<unscoped>)
]

rails test /home/runner/work/newrelic-ruby-agent/newrelic-ruby-agent/test/new_relic/control/security_interface_test.rb:64

fallwith and others added 5 commits October 24, 2023 14:45
- do not call `#source` on a proc, cheat with a source code parsing hack
  instead
- DRY up some shared logic
dinsley and others added 4 commits October 25, 2023 13:34
…o make it clear that its a global attributes context
…cumentation

Updated documentation for NewRelic::Agent#add_custom_log_attributes method to be more clear
ActiveRecord subscriber tests: fixes, cleanup
@github-actions
Copy link
Contributor

SimpleCov Report

Coverage Threshold
Line 94.21% 94%
Branch 85.81% 85%

@fallwith fallwith dismissed kaylareopelle’s stale review October 26, 2023 23:21

Requested changes confirmed

@hannahramadan hannahramadan merged commit 3cff631 into k2 Oct 27, 2023
25 checks passed
@hannahramadan hannahramadan deleted the supportability_metrics branch October 27, 2023 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants