Skip to content
This repository has been archived by the owner on Apr 23, 2020. It is now read-only.

Commit

Permalink
Fixed issue where if the server was down for a time, then came back u…
Browse files Browse the repository at this point in the history 
…p, the plugin would be unable to successfully send data unless the service was restarted.

This was caused by our Duration being greater than the allowed maximum, and never being reset.
This should fix the issue described in https://support.newrelic.com/tickets/55385 and https://support.newrelic.com/tickets/55367?col=2117133&page=1
  • Loading branch information
@jstromwick
jstromwick committed Sep 15, 2013
1 parent 372ad47 commit c401954
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 17 deletions.
23 changes: 23 additions & 0 deletions src/NewRelic.Microsoft.SqlServer.Plugin.Tests/SqlEndpointTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,29 @@ public IEnumerable<TestCaseData> ComponentGuidTestCases
}
}

//Tests fix for issue where Plugin gets 400's after server is unavailable for a time https://support.newrelic.com/tickets/55385
[Test]
[TestCase(1, TestName = "A Minute Since Success")]
[TestCase(5, TestName = "5 Minutes Since Success")]
[TestCase(30, TestName = "30 Minutes Since Success")]
[TestCase(60, TestName = "An Hour Since Success")]
[TestCase(120, TestName = "Two Hours Since Success")]
[TestCase(1440, TestName = "A Day Since Success")]
[TestCase(2880, TestName = "Two Days Since Success")]
public void Assert_duration_does_not_exceed_allowed_max(int minutesSinceLastSuccessful)
{
var endpoint = new SqlServerEndpoint("Foo",".",false);
endpoint.MetricReportSuccessful(DateTime.Now.AddMinutes(minutesSinceLastSuccessful * -1));

const int thirtyMinutesInSeconds = 30 * 60;
Assert.That(endpoint.Duration, Is.LessThanOrEqualTo(thirtyMinutesInSeconds), "Duration should never be longer than 30 minutes, regardless of last succssful reported time");

endpoint.MetricReportSuccessful(DateTime.Now.AddMinutes(-.5));
Assert.That(endpoint.Duration, Is.LessThanOrEqualTo(31), "Duration should reset to be around 30 seconds regardless of previous value");

}

[Test]
public void Assert_endpoint_appropriately_massages_duplicated_data()
{
var endpoint = Substitute.For<SqlEndpointBase>("", "");
Expand Down
55 changes: 38 additions & 17 deletions src/NewRelic.Microsoft.SqlServer.Plugin/SqlEndpointBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,22 @@
using System.Linq;
using System.Text;

using log4net;

using NewRelic.Microsoft.SqlServer.Plugin.Configuration;
using NewRelic.Microsoft.SqlServer.Plugin.Core.Extensions;
using NewRelic.Microsoft.SqlServer.Plugin.Properties;
using NewRelic.Microsoft.SqlServer.Plugin.QueryTypes;
using NewRelic.Platform.Binding.DotNET;

using log4net;

namespace NewRelic.Microsoft.SqlServer.Plugin
{
public abstract class SqlEndpointBase : ISqlEndpoint
{
/// <summary>
/// Metrics with a Duration greater than this value will be rejected by the server (400)
/// </summary>
private const int MaximumAllowedDuration = 1800;
private static readonly ILog _ErrorDetailOutputLogger = LogManager.GetLogger(Constants.ErrorDetailLogger);
private static readonly ILog _VerboseSqlOutputLogger = LogManager.GetLogger(Constants.VerboseSqlLogger);

Expand Down Expand Up @@ -54,7 +58,14 @@ public string[] IncludedDatabaseNames

public int Duration
{
get { return (int) DateTime.Now.Subtract(_lastSuccessfulReportTime).TotalSeconds; }
get
{
var secondsSinceLastSuccessfulReport = (int) DateTime.Now.Subtract(_lastSuccessfulReportTime).TotalSeconds;

return secondsSinceLastSuccessfulReport <= MaximumAllowedDuration
? secondsSinceLastSuccessfulReport
: MaximumAllowedDuration;
}
}

public void SetQueries(IEnumerable<SqlQuery> queries)
Expand All @@ -77,7 +88,7 @@ public void UpdateHistory(IQueryContext[] queryContexts)
{
queryContexts.ForEach(queryContext =>
{
var queryHistory = QueryHistory.GetOrCreate(queryContext.QueryName);
Queue<IQueryContext> queryHistory = QueryHistory.GetOrCreate(queryContext.QueryName);
if (queryHistory.Count >= 2) //Only track up to last run of this query
{
queryHistory.Dequeue();
Expand All @@ -90,8 +101,8 @@ public PlatformData GeneratePlatformData(AgentData agentData)
{
var platformData = new PlatformData(agentData);

var pendingComponentData = QueryHistory.Select(qh => ComponentDataRetriever.GetData(qh.Value.ToArray()))
.Where(c => c != null).ToArray();
ComponentData[] pendingComponentData = QueryHistory.Select(qh => ComponentDataRetriever.GetData(qh.Value.ToArray()))
.Where(c => c != null).ToArray();

pendingComponentData.ForEach(platformData.AddComponent);

Expand All @@ -110,12 +121,13 @@ public virtual void ToLog(ILog log)
log.InfoFormat(" {0}: {1}", Name, safeConnectionString);

// Validate that connection string do not provide both Trusted Security AND user/password
var hasUserCreds = !string.IsNullOrEmpty(safeConnectionString.UserID) || !string.IsNullOrEmpty(safeConnectionString.Password);
bool hasUserCreds = !string.IsNullOrEmpty(safeConnectionString.UserID) || !string.IsNullOrEmpty(safeConnectionString.Password);
if (safeConnectionString.IntegratedSecurity == hasUserCreds)
{
log.Error("==================================================");
log.ErrorFormat("Connection string for '{0}' may not contain both Integrated Security and User ID/Password credentials. " +
"Review the readme.md and update the config file.", safeConnectionString.DataSource);
"Review the readme.md and update the config file.",
safeConnectionString.DataSource);
log.Error("==================================================");
}
}
Expand All @@ -133,7 +145,7 @@ protected IEnumerable<IQueryContext> ExecuteQueries(SqlQuery[] queries, string c

using (var conn = new SqlConnection(connectionString))
{
foreach (var query in queries)
foreach (SqlQuery query in queries)
{
object[] results;
try
Expand All @@ -159,12 +171,15 @@ protected IEnumerable<IQueryContext> ExecuteQueries(SqlQuery[] queries, string c
protected static void LogVerboseSqlResults(ISqlQuery query, IEnumerable<object> results)
{
// This could be slow, so only proceed if it actually gets logged
if (!_VerboseSqlOutputLogger.IsInfoEnabled) return;

if (!_VerboseSqlOutputLogger.IsInfoEnabled)
{
return;
}

var verboseLogging = new StringBuilder();
verboseLogging.AppendFormat("Executed {0}", query.ResourceName).AppendLine();

foreach (var result in results)
foreach (object result in results)
{
verboseLogging.AppendLine(result.ToString());
}
Expand Down Expand Up @@ -197,15 +212,15 @@ internal object[] CalculateSqlDmlActivityIncrease(object[] inputResults, ILog lo
return inputResults;
}

var sqlDmlActivities = inputResults.OfType<SqlDmlActivity>().ToArray();
SqlDmlActivity[] sqlDmlActivities = inputResults.OfType<SqlDmlActivity>().ToArray();

if (!sqlDmlActivities.Any())
{
log.Error("In trying to Process results for SqlDmlActivity, results were NULL or not of the appropriate type");
return inputResults;
}

var currentValues = sqlDmlActivities
Dictionary<string, SqlDmlActivity> currentValues = sqlDmlActivities
.GroupBy(a => string.Format("{0}:{1}:{2}:{3}", BitConverter.ToString(a.PlanHandle), BitConverter.ToString(a.SqlStatementHash), a.CreationTime.Ticks, a.QueryType))
.Select(a => new
{
Expand Down Expand Up @@ -246,7 +261,10 @@ internal object[] CalculateSqlDmlActivityIncrease(object[] inputResults, ILog lo
increase = a.Value.ExecutionCount - previous.ExecutionCount;

// Only record positive deltas, though theoretically impossible here
if (increase <= 0) return;
if (increase <= 0)
{
return;
}
}
else
{
Expand Down Expand Up @@ -282,14 +300,17 @@ internal object[] CalculateSqlDmlActivityIncrease(object[] inputResults, ILog lo
{
Reads = reads,
Writes = writes,
},
}
};
}

private void LogErrorSummary(ILog log, Exception e, ISqlQuery query)
{
var sqlException = e.InnerException as SqlException;
if (sqlException == null) return;
if (sqlException == null)
{
return;
}

log.LogSqlException(sqlException, query, ConnectionString);
}
Expand Down

0 comments on commit c401954

Please sign in to comment.