Skip to content

Commit

Permalink
[999795] Updates to logging (#394)
Browse files Browse the repository at this point in the history
* [999795] Updates to logging

* Resolve review feedback
  • Loading branch information
cormacpayne authored Oct 22, 2019
1 parent cd9a576 commit 1e35ad7
Show file tree
Hide file tree
Showing 18 changed files with 71 additions and 89 deletions.
31 changes: 4 additions & 27 deletions src/BuildScriptGenerator/DefaultBuildScriptGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void GenerateBashScript(
{
// TODO: Should an UnsupportedLanguageException be thrown here?
// Seeing as the issue was that platforms were IDENTIFIED, but no build snippets were emitted from them
LogAndThrowNoPlatformFound(context);
throw new UnsupportedLanguageException(Labels.UnableToDetectLanguageMessage);
}
}

Expand Down Expand Up @@ -306,15 +306,15 @@ private void ThrowInvalidLanguageProvided(BuildScriptGeneratorContext context)
var languages = _programmingPlatforms.Select(sg => sg.Name);
var exc = new UnsupportedLanguageException($"'{context.Language}' platform is not supported. " +
$"Supported platforms are: {string.Join(", ", languages)}");
_logger.LogError(exc, "Exception caught");
_logger.LogError(exc, $"Exception caught, the given platform '{context.Language}' is not supported.");
throw exc;
}

private void LogScriptIfGiven(string type, string scriptPath)
{
if (!string.IsNullOrWhiteSpace(scriptPath))
{
_logger.LogInformation("Using {type} script from {scriptPath}", type, scriptPath);
_logger.LogInformation("Using {type} script", type);
}
}

Expand Down Expand Up @@ -368,29 +368,6 @@ private string BuildScriptFromSnippets(
return script;
}

/// <summary>
/// Handles the error when no platform was found, logging information about the repo.
/// </summary>
private void LogAndThrowNoPlatformFound(BuildScriptGeneratorContext context)
{
try
{
var directoryStructureData = OryxDirectoryStructureHelper.GetDirectoryStructure(
context.SourceRepo.RootPath);
_logger.LogTrace(
"logDirectoryStructure",
new Dictionary<string, string> { { "directoryStructure", directoryStructureData } });
}
catch (Exception ex)
{
_logger.LogError(ex, "Exception caught");
}
finally
{
throw new UnsupportedLanguageException(Labels.UnableToDetectLanguageMessage);
}
}

/// <summary>
/// Gets a matching version for the platform given a version in SemVer format.
/// If the given version is not supported, an exception is thrown.
Expand All @@ -406,7 +383,7 @@ private string GetMatchingTargetVersion(IProgrammingPlatform platform, string ta
if (string.IsNullOrEmpty(maxSatisfyingVersion))
{
var exc = new UnsupportedVersionException(platform.Name, targetVersionSpec, platform.SupportedVersions);
_logger.LogError(exc, "Exception caught");
_logger.LogError(exc, $"Exception caught, the given version '{targetVersionSpec}' is not supported for platform '{platform.Name}'.");
throw exc;
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Linq;
using Microsoft.Extensions.Logging;
using Microsoft.Oryx.BuildScriptGenerator.Exceptions;
using Microsoft.Oryx.Common.Extensions;

namespace Microsoft.Oryx.BuildScriptGenerator
{
Expand Down Expand Up @@ -95,8 +96,9 @@ internal void ReadSettingsFromFile(IDictionary<string, string> settings)
return;
}

foreach (var line in lines)
for (int idx = 0; idx < lines.Length;idx++)
{
var line = lines[idx];
// Ignore comments and blank lines
if (line.StartsWith("#") || string.IsNullOrEmpty(line))
{
Expand All @@ -111,7 +113,7 @@ internal void ReadSettingsFromFile(IDictionary<string, string> settings)
else
{
_logger.LogDebug(
$"Ignoring invalid line '{line}' in '{Constants.BuildEnvironmentFileName}' file.");
$"Ignoring invalid line number '{idx + 1}' in '{Constants.BuildEnvironmentFileName.Hash()}' file.");
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/BuildScriptGenerator/DefaultRunScriptGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ private string RunStartupScriptGeneratorForPlatform(IProgrammingPlatform plat, R

if (exitCode != ProcessConstants.ExitSuccess)
{
_logger.LogError("{scriptGenPath} returned {exitCode}", scriptGenPath, exitCode);
throw new Exception("{scriptGenPath} failed");
_logger.LogError("Generated run script returned exit code '{exitCode}'", exitCode);
throw new Exception($"{scriptGenPath} failed");
}

return File.ReadAllText(_tempScriptPath);
Expand Down
30 changes: 2 additions & 28 deletions src/BuildScriptGenerator/DefaultScriptExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,34 +30,10 @@ public int ExecuteScript(
int exitCode = ProcessHelper.TrySetExecutableMode(scriptPath, workingDirectory);
if (exitCode != ProcessConstants.ExitSuccess)
{
_logger.LogError(
"Failed to set execute permission on script {scriptPath} ({exitCode})",
scriptPath,
exitCode);
return exitCode;
}

exitCode = ExecuteScriptInternal(scriptPath, args, workingDirectory, stdOutHandler, stdErrHandler);
if (exitCode != ProcessConstants.ExitSuccess)
{
try
{
var directoryStructureData = OryxDirectoryStructureHelper.GetDirectoryStructure(workingDirectory);
_logger.LogTrace(
"logDirectoryStructure",
new Dictionary<string, string> { { "directoryStructure", directoryStructureData } });
}
catch (Exception ex)
{
_logger.LogError(ex, "Exception caught");
}
finally
{
_logger.LogError("Execution of script {scriptPath} failed ({exitCode})", scriptPath, exitCode);
}
}

return exitCode;
return ExecuteScriptInternal(scriptPath, args, workingDirectory, stdOutHandler, stdErrHandler);
}

protected virtual int ExecuteScriptInternal(
Expand All @@ -68,9 +44,7 @@ protected virtual int ExecuteScriptInternal(
DataReceivedEventHandler stdErrHandler)
{
int exitCode;
using (var timedEvent = _logger.LogTimedEvent(
"ExecuteScript",
new Dictionary<string, string> { { "scriptPath", scriptPath } }))
using (var timedEvent = _logger.LogTimedEvent("ExecuteScript"))
{
exitCode = ProcessHelper.RunProcess(
scriptPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Oryx.BuildScriptGenerator.Exceptions;
using Microsoft.Oryx.Common.Extensions;
using Newtonsoft.Json;

namespace Microsoft.Oryx.BuildScriptGenerator.DotNetCore
Expand Down Expand Up @@ -52,7 +53,7 @@ public LanguageDetectorResult Detect(ScriptGeneratorContext context)
if (string.IsNullOrEmpty(targetFramework))
{
_logger.LogDebug(
$"Could not find 'TargetFramework' element in the project file '{projectFile}'.");
$"Could not find 'TargetFramework' element in the project file.");
return null;
}

Expand Down Expand Up @@ -137,7 +138,7 @@ private string VerifyAndResolveVersion(string version)
DotNetCoreConstants.LanguageName,
version,
_versionProvider.SupportedDotNetCoreVersions);
_logger.LogError(exc, "Exception caught");
_logger.LogError(exc, $"Exception caught, the given version '{version}' is not supported for the .NET Core platform.");
throw exc;
}

Expand All @@ -161,7 +162,7 @@ private dynamic GetGlobalJsonObject(ISourceRepo sourceRepo)
// in the package.json file.
_logger.LogError(
ex,
$"An error occurred while trying to deserialize {DotNetCoreConstants.GlobalJsonFileName}");
$"An error occurred while trying to deserialize {DotNetCoreConstants.GlobalJsonFileName.Hash()}");
}

return globalJson;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Oryx.BuildScriptGenerator.Exceptions;
using Microsoft.Oryx.Common.Extensions;

namespace Microsoft.Oryx.BuildScriptGenerator.DotNetCore
{
Expand Down Expand Up @@ -43,11 +44,11 @@ public string GetRelativePathToProjectFile(ScriptGeneratorContext context)
var projectFile = Path.Combine(context.SourceRepo.RootPath, projectFileWithRelativePath);
if (context.SourceRepo.FileExists(projectFile))
{
_logger.LogDebug($"Using the project file '{projectFile}' to build.");
_logger.LogDebug($"Using the given .NET Core project file to build.");
}
else
{
_logger.LogWarning($"Could not find the project file '{projectFile}'.");
_logger.LogWarning($"Could not find the .NET Core project file.");
throw new InvalidUsageException(
string.Format(Resources.Labels.DotNetCoreCouldNotFindProjectFileToBuild, projectFile));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Oryx.BuildScriptGenerator.Exceptions;
using Microsoft.Oryx.BuildScriptGenerator.Resources;
using Microsoft.Oryx.Common.Extensions;

namespace Microsoft.Oryx.BuildScriptGenerator.DotNetCore
{
Expand Down Expand Up @@ -93,9 +94,7 @@ public string GetRelativePathToProjectFile(ScriptGeneratorContext context)

if (projectFile == null)
{
_logger.LogDebug(
$"Could not find a project file to build. Available project files: " +
$"{string.Join(',', allProjects)}");
_logger.LogDebug("Could not find a .NET Core project file to build.");
return null;
}

Expand Down
3 changes: 2 additions & 1 deletion src/BuildScriptGenerator/Node/NodeDependencyChecker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.Extensions.Logging;
using Microsoft.Oryx.Common.Extensions;

namespace Microsoft.Oryx.BuildScriptGenerator.Node
{
Expand All @@ -32,7 +33,7 @@ public IEnumerable<ICheckerMessage> CheckSourceRepo(ISourceRepo repo)
dynamic packageJson = NodePlatform.GetPackageJsonObject(repo, null);
if (packageJson == null)
{
_logger.LogDebug("packageJson is null; skipping checking for superseded packages");
_logger.LogDebug($"{NodeConstants.PackageJsonFileName.Hash()} is null; skipping checking for superseded packages");
return Enumerable.Empty<ICheckerMessage>();
}

Expand Down
5 changes: 3 additions & 2 deletions src/BuildScriptGenerator/Node/NodeLanguageDetector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Oryx.BuildScriptGenerator.Exceptions;
using Microsoft.Oryx.Common.Extensions;

namespace Microsoft.Oryx.BuildScriptGenerator.Node
{
Expand Down Expand Up @@ -87,7 +88,7 @@ public LanguageDetectorResult Detect(ScriptGeneratorContext context)
{
_logger.LogDebug(
"App in repo is not a Node.js app as it has the file {iisStartupFile}",
iisStartupFile);
iisStartupFile.Hash());
return null;
}
}
Expand Down Expand Up @@ -141,7 +142,7 @@ private string DetectNodeVersion(dynamic packageJson)
NodeConstants.NodeJsName,
nodeVersionRange,
_versionProvider.SupportedNodeVersions);
_logger.LogError(exc, "Exception caught");
_logger.LogError(exc, $"Exception caught, the version '{nodeVersionRange}' is not supported for the node platform.");
throw exc;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/BuildScriptGenerator/Node/NodePlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public void SetRequiredTools(
}
else
{
_logger.LogDebug($"packageJson is null; skipping setting {NodeConstants.NpmToolName} tool");
_logger.LogDebug($"{NodeConstants.PackageJsonFileName} is null; skipping setting {NodeConstants.NpmToolName} tool");
}
}

Expand Down Expand Up @@ -319,7 +319,7 @@ internal static dynamic GetPackageJsonObject(ISourceRepo sourceRepo, ILogger log
// This prevents Oryx from erroring out when Node.js itself might be able to tolerate the file.
logger.LogWarning(
exc,
$"Exception caught while trying to deserialize {NodeConstants.PackageJsonFileName}");
$"Exception caught while trying to deserialize {NodeConstants.PackageJsonFileName.Hash()}");
}

return packageJson;
Expand Down
5 changes: 3 additions & 2 deletions src/BuildScriptGenerator/Php/PhpLanguageDetector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.Extensions.Options;
using Microsoft.Oryx.BuildScriptGenerator.Exceptions;
using Microsoft.Oryx.BuildScriptGenerator.SourceRepo;
using Microsoft.Oryx.Common.Extensions;

namespace Microsoft.Oryx.BuildScriptGenerator.Php
{
Expand Down Expand Up @@ -51,7 +52,7 @@ public LanguageDetectorResult Detect(ScriptGeneratorContext context)
// some errors in the composer.json file.
_logger.LogWarning(
ex,
$"Exception caught while trying to deserialize {PhpConstants.ComposerFileName}");
$"Exception caught while trying to deserialize {PhpConstants.ComposerFileName.Hash()}");
}

string runtimeVersion = VerifyAndResolveVersion(composerFile?.require?.php?.Value as string);
Expand All @@ -78,7 +79,7 @@ private string VerifyAndResolveVersion(string version)
PhpConstants.PhpName,
version,
_versionProvider.SupportedPhpVersions);
_logger.LogError(exc, "Exception caught");
_logger.LogError(exc, $"Exception caught, the version '{version}' is not supported for the PHP platform.");
throw exc;
}

Expand Down
3 changes: 2 additions & 1 deletion src/BuildScriptGenerator/Php/PhpPlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.Extensions.Options;
using Microsoft.Oryx.BuildScriptGenerator.SourceRepo;
using Microsoft.Oryx.Common;
using Microsoft.Oryx.Common.Extensions;

namespace Microsoft.Oryx.BuildScriptGenerator.Php
{
Expand Down Expand Up @@ -66,7 +67,7 @@ public BuildScriptSnippet GenerateBashBuildScriptSnippet(BuildScriptGeneratorCon
{
// Leave malformed composer.json files for Composer to handle.
// This prevents Oryx from erroring out when Composer itself might be able to tolerate the file.
_logger.LogWarning(exc, $"Exception caught while trying to deserialize {PhpConstants.ComposerFileName}");
_logger.LogWarning(exc, $"Exception caught while trying to deserialize {PhpConstants.ComposerFileName.Hash()}");
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/BuildScriptGenerator/Python/PythonLanguageDetector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Oryx.BuildScriptGenerator.Exceptions;
using Microsoft.Oryx.Common.Extensions;

namespace Microsoft.Oryx.BuildScriptGenerator.Python
{
Expand Down Expand Up @@ -82,7 +83,7 @@ private string VerifyAndResolveVersion(string version)
PythonConstants.PythonName,
version,
_versionProvider.SupportedPythonVersions);
_logger.LogError(exc, "Exception caught");
_logger.LogError(exc, $"Exception caught, the version '{version}' is not supported for the Python platform.");
throw exc;
}

Expand All @@ -107,7 +108,7 @@ private string DetectPythonVersionFromRuntimeFile(ISourceRepo sourceRepo)
_logger.LogDebug(
"Prefix {verPrefix} was not found in file {rtFileName}",
versionPrefix,
PythonConstants.RuntimeFileName);
PythonConstants.RuntimeFileName.Hash());
return null;
}

Expand All @@ -120,7 +121,7 @@ private string DetectPythonVersionFromRuntimeFile(ISourceRepo sourceRepo)
_logger.LogError(
ex,
"An error occurred while reading file {rtFileName}",
PythonConstants.RuntimeFileName);
PythonConstants.RuntimeFileName.Hash());
}
}
else
Expand Down
11 changes: 5 additions & 6 deletions src/BuildScriptGeneratorCli/Commands/BuildCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,10 @@ internal int Execute(
// Write build script to selected path
File.WriteAllText(buildScriptPath, scriptContent);
logger.LogTrace("Build script written to file");
logger.LogDebug("Build script content:\n" + scriptContent);
if (DebugMode)
{
console.WriteLine($"Build script content:\n{scriptContent}");
}

var buildEventProps = new Dictionary<string, string>()
{
Expand Down Expand Up @@ -270,8 +273,6 @@ internal int Execute(
timedEvent.AddProperty("exitCode", exitCode.ToString());
}

logger.LogLongMessage(LogLevel.Debug, "Build script output", buildScriptOutput.ToString());

if (exitCode != ProcessConstants.ExitSuccess)
{
logger.LogError("Build script exited with {exitCode}", exitCode);
Expand Down Expand Up @@ -312,9 +313,7 @@ internal override bool IsValidInput(IServiceProvider serviceProvider, IConsole c
if (IsSubDirectory(options.IntermediateDir, options.SourceDir))
{
logger.LogError(
"Intermediate directory {intermediateDir} cannot be a child of {srcDir}",
options.IntermediateDir,
options.SourceDir);
"Intermediate directory cannot be a child of the source directory.");
console.WriteErrorLine(
$"Intermediate directory '{options.IntermediateDir}' cannot be a " +
$"sub-directory of source directory '{options.SourceDir}'.");
Expand Down
Loading

0 comments on commit 1e35ad7

Please sign in to comment.