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 manual rate limiting and fix GlobalTracer reset #22

Merged
merged 2 commits into from
Jan 15, 2024

Conversation

allenisalai
Copy link
Contributor

@allenisalai allenisalai commented Jan 10, 2024

The sample rate is sent to the Datadog agent and is used even when manual profiling like this project does so the sampling logic in the DataDogProfiler is causing the sampling rate to be applied twice.

In addition the Global tracer isn't being reset properly in stopAndIgnore which creates a single trace even when multiple traces are subsequently started and stopped.

#[AsCommand('app:profiler-test')]
class ProfilerTestCommand extends Command
{
    public function __construct(private ProfilerInterface $profiler)
    {
        parent::__construct();
    }

    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        $this->profiler->stopAndIgnore();

        for ($i = 0; $i < 5; $i++) {
            $this->profiler->start('test');
            $output->writeln((string) $i);
            usleep(50000);
            $this->profiler->stop();
        }
        return Command::SUCCESS;
    }
}

There are no traces on master. In the screenshot you can see 2 requests since I ran the command twice, but no spans were created.
Screenshot 2024-01-11 at 7 47 42 AM

There are 4 traces using this branch:
Screenshot 2024-01-11 at 7 31 49 AM

@magikid
Copy link
Member

magikid commented Jan 11, 2024

The test failures might be related to DataDog/dd-trace-php#944

@allenisalai
Copy link
Contributor Author

DataDog/dd-trace-php#1533

Its not part of the interface. I'll see if they have a suggestion to match our use case.

@allenisalai
Copy link
Contributor Author

ini_set("datadog.tracer.enabled", '0');
ini_set("datadog.tracer.enabled", '1');

Doesn't work. For now I'm going to remove the stopAndIgnore changes from the PR since there's no clear resolution yet.

@allenisalai allenisalai force-pushed the fix-datadog-tracer-reset branch 2 times, most recently from a7a5a13 to 4e13023 Compare January 11, 2024 19:13
@allenisalai
Copy link
Contributor Author

Keeping GlobalTracer::set(new Tracer()) worked in conjunction with disabling and reenabling dd tracer.

@magikid magikid merged commit 44ae175 into sourceability:main Jan 15, 2024
7 checks passed
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