Skip to content

Add debuggability feature for specific request with debug key #14

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: task/check-dd-extensions
Choose a base branch
from
Open

Add debuggability feature for specific request with debug key #14

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/BuffLog/BuffLog.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ class BuffLog {

// verbosity can be changed with setting this env var
public static $logLevelEnvVar = "LOG_LEVEL";
public static $isDebug = false;
public static $debugKey = "debug";
public static $debugValue = "";

// we can use strtolower(Logger::getLevels()) instead
private static $logOutputMethods = ['debug', 'info', 'notice', 'warning', 'error', 'critical'];
Expand Down Expand Up @@ -49,6 +52,7 @@ protected static function configureInstance()

$logLevelFromEnv = getenv(self::$logLevelEnvVar);
$monologLevels = $logger->getLevels();

if ($logLevelFromEnv) {
// only if the level exists, we change the verbosity level
if (key_exists($logLevelFromEnv, $monologLevels)) {
Expand All @@ -58,6 +62,8 @@ protected static function configureInstance()
}
}

self::checkDebug();

$handler = new StreamHandler('php://stdout', self::$verbosityLevel);
$handler->setFormatter( new \Monolog\Formatter\JsonFormatter() );
$logger->pushHandler($handler);
Expand Down Expand Up @@ -86,6 +92,18 @@ public static function __callStatic($methodName, $args)
}
}

private static function checkDebug()
{
if (is_array($_GET) && isset($_GET[self::$debugKey])) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should we take into account only GET requests or also POST ones?

Copy link

Choose a reason for hiding this comment

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

I think POST too - particularly for API requests (eg creating an update) it would be useful to get the trace in those situations too as well as the 'read' requests.

self::$isDebug = true;
// we set the lowest level of logs to see everything
self::$verbosityLevel = Logger::DEBUG;
self::$debugValue = $_GET[self::$debugKey];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we should probably sanitize this

Copy link

@colinscape colinscape Dec 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

There may well be security aspects to consider too. Since many API endpoints will be publicly available, there's the potential for anyone to include this key in their request, which could have the effect of making us incur large logging costs! 😅

It's not a huge risk, but it does exist. Potentially something like the API Gateway could help here by only letting this parameter through if the request comes via the VPN (or I guess from 'inside' the cluster too).

As a proof of concept, I think this is fine as is, but we might want to keep this in mind that it is something of a half-open door if someone wants to act maliciously against us.

return true;
}
return false;
}

private static function enrichLog()
{
// We should probably implement this as a Monolog Processor
Expand All @@ -110,6 +128,10 @@ private static function enrichLog()
"span_id" => $ddTraceSpan->getSpanId()
];

if (self::$isDebug) {
$record['context']['debug'] = self::$debugValue;
}

} catch (Exception $e) {
error_log($e->getMessage() . " Can't add trace to logs. Have you setup the Datadog Tracer extension? If you run a worker have your added the DD_TRACE_CLI_ENABLED env variable?");
}
Expand Down