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

Extract runtime metrics worker from Writer #1004

Merged
merged 1 commit into from
Apr 15, 2020

Conversation

delner
Copy link
Contributor

@delner delner commented Apr 14, 2020

In a continuation of the shift towards immutable components, this pull request extracts the runtime metrics worker out of the writer. Previously this has been nested in tracer.writer.runtime_metrics, and has piggybacked on the writer's thread to do runtime metrics work.

To do this:

  • Runtime metrics behaviors are completely removed from the existing Datadog::Writer.
  • runtime_metrics is introduced as an attribute of Components, which is a Datadog::Workers::RuntimeMetrics.
    • This contains its own worker thread which collects and emits runtime metrics on a polling basis.

@delner delner added core Involves Datadog core libraries dev/refactor Involves refactoring existing components labels Apr 14, 2020
@delner delner added this to the 0.35.0 milestone Apr 14, 2020
@delner delner requested review from marcotc and a team April 14, 2020 17:08
@delner delner self-assigned this Apr 14, 2020
lib/ddtrace/configuration.rb Outdated Show resolved Hide resolved
logger = settings.logger.instance || Datadog::Logger.new(STDOUT)
logger.level = settings.diagnostics.debug ? ::Logger::DEBUG : settings.logger.level
def build_runtime_metrics_worker(settings)
# NOTE: Should we just ignore building the worker if its not enabled?
Copy link
Member

Choose a reason for hiding this comment

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

For the time being, because we still allow dynamic reconfiguration, I think always initializing it is ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll remove this for now... we'll consider optimizations like this later.

Copy link
Member

@marcotc marcotc Apr 15, 2020

Choose a reason for hiding this comment

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

I think the comment is actually helpful for future work towards true immutable components: it guides us to where we need to make changes.
I think we should keep it.

I think I saw the comment as a "request for comment" and I was just chiming in on what I think about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that is kind of the spirit of writing it, so this is good.

@delner delner force-pushed the refactor/extract_runtime_metrics_worker branch from d4178f8 to 10592ce Compare April 15, 2020 21:08
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.

🥇

@delner delner merged commit 3c342d5 into master Apr 15, 2020
@delner delner deleted the refactor/extract_runtime_metrics_worker branch April 15, 2020 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries dev/refactor Involves refactoring existing components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants