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

Add new configuration option to disable ADP #11513

Merged
merged 5 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Binary file modified extern/Analytics.NET/AdpSDKCSharpWrapper.dll
Binary file not shown.
Binary file modified extern/Analytics.NET/Analytics.Net.ADP.dll
Binary file not shown.
31 changes: 28 additions & 3 deletions src/DynamoCore/Logging/AnalyticsService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,40 @@ namespace Dynamo.Logging
/// </summary>
class AnalyticsService
{
private static IAnalyticsUI adpAnalyticsUI = new ADPAnalyticsUI();
private static ADPAnalyticsUI adpAnalyticsUI = new ADPAnalyticsUI();

/// <summary>
/// Starts the client when DynamoModel is created. This method initializes
/// the Analytics service and application life cycle start is tracked.
/// </summary>
/// <param name="model">DynamoModel</param>
internal static void Start(DynamoModel model)
/// <param name="disableADP">Pass true to disable ADP for the lifetime of the Dynamo or host process</param>
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="isHeadless">Analytics won't be started if IsHeadless, but ADP may be loaded to be disabled.</param>
/// <param name="isTestMode">Analytics won't be started if isTestMode, ADP will not be loaded.</param>
internal static void Start(DynamoModel model, bool disableADP, bool isHeadless, bool isTestMode)
{
if (isTestMode)
{
return;
}
if (disableADP)
{
//if this returns false something has gone wrong.
//the client requested ADP be disabled, but we cannot disable it.
if (!adpAnalyticsUI.DisableADPForProcessLifetime())
{
//TODO consider throwing instead - that will cause a crash.
model.Logger.LogNotification("Dynamo",
"Dynamo Startup Error",
"Analytics could not be disabled",
"Dynamo was started with configuration requesting ADP be disabled - but ADP could not be disabled.");
}
}

if (isHeadless)
Copy link
Member Author

Choose a reason for hiding this comment

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

I modified how late we bail out if isHeadless is true because some clients need to disable ADP and still use Dynamo as a headless runner.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remind me, the isHeadless is set to true only when using command line, right? For Dynamo Player and GD, analytics is on because we are using Dynamo as a server.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe player does not use headless, I am not sure about GD.

{
return;
}
var client = new DynamoAnalyticsClient(model);
Analytics.Start(client);
model.WorkspaceAdded += OnWorkspaceAdded;
Expand All @@ -35,7 +60,7 @@ static void OnWorkspaceAdded(WorkspaceModel obj)
/// <summary>
/// Indicates whether the user has opted-in to ADP analytics.
/// </summary>
internal static bool IsADPOptedIn
internal static bool IsADPOptedIn
{
get
{
Expand Down
28 changes: 21 additions & 7 deletions src/DynamoCore/Models/DynamoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ public IEnumerable<WorkspaceModel> Workspaces
public AuthenticationManager AuthenticationManager { get; set; }

internal static string DefaultPythonEngine { get; private set; }

private bool disableADPForProcess;
#endregion

#region initialization and disposal
Expand Down Expand Up @@ -555,6 +555,7 @@ public struct DefaultStartConfiguration : IStartConfiguration
/// Default Python script engine
/// </summary>
public string DefaultPythonEngine { get; set; }
public bool DisableADPForProcess { get; set; }
}

/// <summary>
Expand Down Expand Up @@ -586,11 +587,13 @@ public static DynamoModel Start(IStartConfiguration configuration)
/// <param name="config">Start configuration</param>
protected DynamoModel(IStartConfiguration config)
{
if (config is DefaultStartConfiguration)
if (config is DefaultStartConfiguration defaultStartConfig)
{
// This is not exposed in IStartConfiguration to avoid a breaking change.
// TODO: This fact should probably be revisited in 3.0.
DefaultPythonEngine = ((DefaultStartConfiguration)config).DefaultPythonEngine;
DefaultPythonEngine = defaultStartConfig.DefaultPythonEngine;
disableADPForProcess = defaultStartConfig.DisableADPForProcess;

}

ClipBoard = new ObservableCollection<ModelBase>();
Expand Down Expand Up @@ -668,6 +671,20 @@ protected DynamoModel(IStartConfiguration config)
{
// Do nothing for now
}

// these configuration options are incompatible, one requires loading ADP binaries
// the other depends on not loading those same binaries.

if (areAnalyticsDisabledFromConfig && disableADPForProcess)
{
throw new ConfigurationException("Incompatible configuration: could not start Dynamo with both [Analytics disabled] and [ADP disabled per process] config options enabled");
}

if(IsTestMode && disableADPForProcess)
{
this.Logger.Log("Incompatible configuration: [IsTestMode] and [ADP disabled per process] ");
}

// If user skipped analytics from assembly config, do not try to launch the analytics client
if (!areAnalyticsDisabledFromConfig)
{
Expand Down Expand Up @@ -1407,10 +1424,7 @@ private void LoadNodeModels(List<TypeLoadData> nodes, bool isPackageMember)

private void InitializeAnalyticsService()
{
if (!IsTestMode && !IsHeadless)
{
AnalyticsService.Start(this);
}
AnalyticsService.Start(this,disableADPForProcess, IsHeadless, IsTestMode);
}

private IPreferences CreateOrLoadPreferences(IPreferences preferences)
Expand Down