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

Remove CI gem dependency #3288

Merged
merged 9 commits into from
Dec 20, 2023
Merged

Remove CI gem dependency #3288

merged 9 commits into from
Dec 20, 2023

Conversation

TonyCTHsu
Copy link
Contributor

@TonyCTHsu TonyCTHsu commented Nov 29, 2023

2.0 Upgrade Guide notes

As of 2.0, ddtrace, our CI visibility component has been extracted as a separate gem named datadog-ci, and will no longer be installed together with ddtrace.

If you are using our CI visibility product, you will need to include datadog-ci in the project's Gemfile (learn more about the setup)

gem 'ddtrace', '>= 2'
gem 'datadog-ci'

If you do not want to install datadog-ci, please ensure any CI related configuration under config.ci.* is removed.

  Datadog.configure do |config|
    config.ci.enabled = true
    
    config.ci.instrument :rspec
    config.ci.instrument :cucumber
  end

What does this PR do?

This PR removes the datadog-ci dependency from ddtrace

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@TonyCTHsu TonyCTHsu added do-not-merge/WIP Not ready for merge breaking-change Involves a breaking change ci-app CI product for test suite instrumentation labels Nov 29, 2023
@TonyCTHsu TonyCTHsu self-assigned this Nov 29, 2023
@github-actions github-actions bot added appsec Application Security monitoring product core Involves Datadog core libraries integrations Involves tracing integrations profiling Involves Datadog profiling tracing and removed ci-app CI product for test suite instrumentation labels Nov 29, 2023
@TonyCTHsu TonyCTHsu changed the base branch from master to drop-ruby-2.1-2.2-2.3-support November 29, 2023 12:03
@github-actions github-actions bot removed tracing appsec Application Security monitoring product integrations Involves tracing integrations profiling Involves Datadog profiling labels Nov 29, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4faaf65) 98.07% compared to head (d329ce3) 98.07%.

Additional details and impacted files
@@           Coverage Diff           @@
##              2.0    #3288   +/-   ##
=======================================
  Coverage   98.07%   98.07%           
=======================================
  Files        1240     1240           
  Lines       71978    71997   +19     
  Branches     3391     3392    +1     
=======================================
+ Hits        70589    70610   +21     
+ Misses       1389     1387    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TonyCTHsu TonyCTHsu marked this pull request as ready for review November 29, 2023 15:13
@TonyCTHsu TonyCTHsu requested review from a team as code owners November 29, 2023 15:13
@ekump
Copy link
Contributor

ekump commented Nov 29, 2023

Just a bit of a nitpick: This PR is against the branch drop-ruby-2.1-2.2-2.3-support. Is that accurate? And if so, is removing CI necessary for dropping 2.1-2.3 support?

@anmarchenko
Copy link
Member

Thank you! only a couple of points:

require 'rspec'
require 'datadog/ci'

# Only activates test instrumentation on CI
if ENV["DD_ENV"] == "ci"
  Datadog.configure do |c|
    # Configures the tracer to ensure results delivery
    c.ci.enabled = true

    # The name of the service or library under test
    c.service = "my-ruby-app"

    # Enables the RSpec instrumentation
    c.ci.instrument :rspec
  end
end

@marcotc marcotc requested a review from a team as a code owner November 30, 2023 21:55
@marcotc marcotc changed the base branch from drop-ruby-2.1-2.2-2.3-support to 2.0 November 30, 2023 21:56
@TonyCTHsu TonyCTHsu removed the do-not-merge/WIP Not ready for merge label Dec 11, 2023
Copy link
Contributor

@alai97 alai97 left a comment

Choose a reason for hiding this comment

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

Looks good for docs!

@@ -145,6 +144,7 @@ def additional_payload_variables
format_configuration_value(configuration.tracing.writer_options[:flush_interval])
options['logger.instance'] = configuration.logger.instance.class.to_s
options['appsec.enabled'] = configuration.dig('appsec', 'enabled') if configuration.respond_to?('appsec')
options['ci.enabled'] = configuration.dig('ci', 'enabled') if configuration.respond_to?('ci')
Copy link
Member

Choose a reason for hiding this comment

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

Is this still relevant @TonyCTHsu @anmarchenko? I think this might have to be moved to the datadog-ci gem instead.

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure what is the meaning of this, maybe it is important for tracer to report the value of ci.enabled to telemetry. Though it will always be false because telemetry is not enabled when CI mode is on, so you might as well remove it

Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

Looks great!
I only left one comment, but overall it's in great shape! #3288 (comment)

@anmarchenko
Copy link
Member

anmarchenko commented Dec 12, 2023

I have only one comment: this section https://github.com/DataDog/dd-trace-rb/blob/master/docs/GettingStarted.md#ci-visibility still exists in dd-trace-rb docs. I would like us to put there text "For CI visibility use the datadog-ci gem".

Copy link
Member

@anmarchenko anmarchenko left a comment

Choose a reason for hiding this comment

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

looks good now, thank you!

@TonyCTHsu TonyCTHsu merged commit 7a23c71 into 2.0 Dec 20, 2023
151 checks passed
@TonyCTHsu TonyCTHsu deleted the tonycthsu/remove-ci-dep branch December 20, 2023 13:29
@TonyCTHsu TonyCTHsu added this to the 2.0 milestone Feb 20, 2024
@ivoanjo ivoanjo added the 2.0 label Mar 14, 2024
@TonyCTHsu TonyCTHsu modified the milestones: 2.0, 2.0.0.beta1 Mar 22, 2024
@TonyCTHsu TonyCTHsu mentioned this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0 breaking-change Involves a breaking change core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants