Adls data plane task add debug functionality #5573

rahuldutta90 · 2018-02-17T00:21:01Z

Description

Add debug functionality to ADLS dataplane cmdlets

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review
- the cmdlet markdown files were generated using the platyPS module

cormacpayne

@rahuldutta90 a few comments to take a look at

cormacpayne · 2018-02-22T00:35:00Z

src/ResourceManager/DataLakeStore/AzureRM.DataLakeStore.psd1

-		* Get-AzureRMDataLakeStoreItemContent - Fixed the tail behavior for contents greater than 4MB
-		* Set-AzureRMDataLakeStoreItemExpiry - Introduced new parameter set SetRelativeExpiry for setting relative expiration time
-		* Remove-AzureRmDataLakeStoreItem (https://github.com/Azure/azure-powershell/blob/adls-data-plane/src/ResourceManager/DataLakeStore/documentation/upcoming-breaking-changes.md) - Deprecated parameter Clean.
+		* Add Debug functionality to Datalake store dataplane cmdlets


@rahuldutta90 you can revert the changes made to the ReleaseNotes -- we will update this metadata when we release this module again

@cormacpayne Reverted it.

cormacpayne · 2018-02-22T00:39:59Z

...anager/DataLakeStore/Commands.DataLakeStore/DataPlaneModels/DataLakeStoreFileSystemClient.cs

+        static DataLakeStoreFileSystemClient()
+        {
+            // ConfigurationItemFactory.Default.Targets.RegisterDefinition("AdlsLogger", typeof(AdlsLoggerTarget));
+            Target.Register<Microsoft.Azure.Commands.DataLakeStore.Models.AdlsLoggerTarget>("AdlsLogger"); //generic


@rahuldutta90 what is the purpose of this target in NLog?

@cormacpayne NLog is used by the ADLS dataplane sdk to log debug messages. We can create a custom target which basically queues the debug data to the ConcurrentQueue (the one available to this powershell framework) for debug messages. So whenever we use our nlog framework to write debug message all of them will get queued in the concurrent queue. I have put some more comments in the target class.

@cormacpayne https://github.com/NLog/NLog/wiki/Extending-NLog

cormacpayne · 2018-02-22T00:40:21Z

...anager/DataLakeStore/Commands.DataLakeStore/DataPlaneModels/DataLakeStoreFileSystemClient.cs

        #region Constructors

+        static DataLakeStoreFileSystemClient()
+        {
+            // ConfigurationItemFactory.Default.Targets.RegisterDefinition("AdlsLogger", typeof(AdlsLoggerTarget));


@rahuldutta90 nit: would you mind removing this line if it's going to be commented-out?

@cormacpayne Removed. Thanks for catching it.

cormacpayne · 2018-02-22T00:40:33Z

...anager/DataLakeStore/Commands.DataLakeStore/DataPlaneModels/DataLakeStoreFileSystemClient.cs

@@ -68,6 +77,22 @@ public DataLakeStoreFileSystemClient(IAzureContext context)
            ServicePointManager.UseNagleAlgorithm = false;
        }

+        public DataLakeStoreFileSystemClient(IAzureContext context, DataLakeStoreFileSystemCmdletBase cmdlet) : this(context)
+        {
+


@rahuldutta90 nit: please remove this blank line

@cormacpayne Removed it.

markcowl · 2018-03-02T03:14:44Z

src/ResourceManager/DataLakeStore/ChangeLog.md

+
+* Add debug functionality
+
+## Version 5.1.1


I don't think you want to change the order of this

@markcowl Addressed it.

markcowl · 2018-03-02T03:19:49Z

...anager/DataLakeStore/Commands.DataLakeStore/DataPlaneModels/DataLakeStoreFileSystemClient.cs

-            fileTarget.Layout = "${message}";
+
+            var rule = new LoggingRule("adls.dotnet.*", NLog.LogLevel.Debug, fileTarget);
+            _adlsLoggerConfig.LoggingRules.Add(rule);


Can you describe what this actually logs? We want to make sure that request method and headers and response status and headers are included

@markcowl This is the debug line NLog posts:

2018-02-16 09:23:59.1720|DEBUG|adls.dotnet.WebTransport|HTTPRequest,Succeeded,cReqId:cc6758eb-656f-4bd7-b589-67165664e899.0,lat:6496,err,Reqlen:0,Resplen:0,token_ns:2,sReqId:2551abc6-ed04-4a99-b71f-264da768cf7d,path:/NewFile,qp:op=CREATE&overwrite=True&leaseid=30038d32-172f-4054-ac08-359421c24282&filesessionid=30038d32-172f-4054-ac08-359421c24282&CreateParent=True&write=true&syncFlag=DATA&api-version=2017-08-01

@markcowl For failure case only we have the httpstatus in response like here we have InternalServorError:
2018-02-14 20:02:48.5660|DEBUG|adls.dotnet.WebTransport|HTTPRequest,failed,cReqId:e1dc5c6d-a9c1-4202-8d99-7e6c00d19f8d.0,lat:723,errInternalServerError: RuntimeException,Reqlen:15,Resplen:0,token_ns:0,sReqId:a2ed5f89-f8a7-42e6-af1c-187806135668,path:/Test/dir1/testConcurrentAppendParallel_15,qp:op=CONCURRENTAPPEND&appendMode=autocreate&api-version=2017-08-01

We have been using the SDK for a while in both Java and .Net. For both of these, we have looked at what customers typically need in order to troubleshoot errors, and that is what we log in our logs. This is what customers have been using to troubleshoot large systems, like Hadoop - provides a good balance between readable and analyzable data, and the huge volume of logs that some of our runs can generate (e.g., recursive ACL setting or large data upload). i.e., this is the best balance (so far) we have found between volume of data and usefulness of data to be able to troubleshoot.

maddieclayton · 2018-03-05T19:11:55Z

@rahuldutta90 Please fix the merge conflicts that have arisen.

cormacpayne · 2018-03-12T18:05:17Z

@rahuldutta90 Hey Rahul, what's the status of this PR? I know that we were looking to re-format the debug log, as well as pull in the latest changes from the preview branch.

rahuldutta90 · 2018-03-12T23:28:44Z

@cormacpayne I am in process of making the changes in SDK. Once it is released, I will integrate that version with this PR.

markcowl · 2018-03-20T00:20:42Z

on demand run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/636/

Approved offline

rahuldutta90 added 4 commits January 31, 2018 18:56

First commit of the target logger

ecc8e6d

Merged from preview

9da683c

Merge adls-data-plane to this branch

c030dd1

Fix the change log and merge issues

b599d75

rahuldutta90 requested review from cormacpayne and markcowl February 17, 2018 00:21

azuresdkci added the needs-review label Feb 17, 2018

azuresdkci assigned cormacpayne Feb 17, 2018

cormacpayne suggested changes Feb 22, 2018

View reviewed changes

cormacpayne added needs-revision and removed needs-review labels Feb 22, 2018

cormacpayne assigned rahuldutta90 and unassigned cormacpayne Feb 22, 2018

Revert the psd1 file, address other comments of review

8cf6bb9

rahuldutta90 assigned asikaria-msft Feb 22, 2018

rahuldutta90 requested a review from asikaria-msft February 22, 2018 01:35

rahuldutta90 unassigned asikaria-msft Feb 22, 2018

Remove unused usings and make the target class internal

2686057

maddieclayton assigned cormacpayne and unassigned rahuldutta90 Feb 26, 2018

maddieclayton added needs-review and removed needs-revision labels Feb 26, 2018

cormacpayne assigned markcowl Mar 1, 2018

markcowl previously requested changes Mar 2, 2018

View reviewed changes

markcowl added needs-revision and removed needs-review labels Mar 2, 2018

markcowl assigned rahuldutta90 and unassigned markcowl and cormacpayne Mar 2, 2018

Change changelog

dd92629

maddieclayton added the Merge Conflicts label Mar 5, 2018

Merge adls-data-plane

104a29c

maddieclayton assigned cormacpayne and unassigned rahuldutta90 Mar 6, 2018

maddieclayton added needs-review and removed Merge Conflicts needs-revision labels Mar 6, 2018

cormacpayne assigned rahuldutta90 and unassigned cormacpayne Mar 12, 2018

cormacpayne added needs-revision and removed needs-review labels Mar 12, 2018

Update the ADLS version to include logging details

97935bd

maddieclayton assigned cormacpayne and unassigned rahuldutta90 Mar 16, 2018

maddieclayton added needs-review and removed needs-revision labels Mar 16, 2018

markcowl added Waiting for On Demand and removed needs-review labels Mar 19, 2018

cormacpayne approved these changes Mar 20, 2018

View reviewed changes

cormacpayne merged commit 26239e0 into adls-data-plane Mar 20, 2018

markcowl deleted the adls-data-plane-task-addDebugFunctionality branch May 1, 2018 17:47

Adls data plane task add debug functionality #5573

Adls data plane task add debug functionality #5573

Uh oh!

Conversation

rahuldutta90 commented Feb 17, 2018 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

cormacpayne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maddieclayton commented Mar 5, 2018

Uh oh!

cormacpayne commented Mar 12, 2018

Uh oh!

rahuldutta90 commented Mar 12, 2018

Uh oh!

markcowl commented Mar 20, 2018

Uh oh!

Uh oh!

rahuldutta90 commented Feb 17, 2018 •

edited by azuresdkci

Loading