microsoft / coe-starter-kit

Other
743 stars 219 forks source link

[CoE Starter Kit - BUG] DLP Editor V2 Impact Analysis Issue - filter for impacted apps and flows is different between DLP Editor canvas app and DLP Editor>Parse impacted resources into CSV flow #2652

Closed JonTaylor5 closed 2 years ago

JonTaylor5 commented 2 years ago

Describe the issue

I am running the DLP Editor V2 in a D4T environment which is checking 1753 apps and 9385 flows. It works fine with analysing the apps but when when I click the Analyzed Flows tabs the 5 dots run across the top of the screen for about 30mins+ before I get what looks like some form of time out. Please see attached

2022-05-06 16_43_38-DLP Editor V2 - Power Apps Page Unresponsive 2022-05-06 16_46_18-DLP Editor V2 - Power Apps Browser Error

.

Expected Behavior

I would expect to see a list of impactedd flows

What solution are you experiencing the issue with?

Core

What solution version are you using?

3.11 Core for Teams

What app or flow are you having the issue with?

DLP Editor V2

Steps To Reproduce

  1. Open DLP Editor V2
  2. Add Environment
  3. Go to Impact Analysis

Anything else?

No response

manuelap-msft commented 2 years ago

Hello,

the DLP Impact Analysis does the work inline using canvas app functions, 9385 flows may just be hitting the limit of what's possible to perform in the memory of your browser / inline with the app. You may have to use the "download a csv file" option instead of looking at the apps inline - this outsources the impact analysis to a flow and then sends you a .csv file with impacted apps and flows. It is independent of the impact analysis working in the app.

Based on the large number of apps and flows you're analyzing, the flow make take some time to run but should succeed.

Thank you Manuela

JonTaylor5 commented 2 years ago

Hi @manuelap-msft , thank you for the update. When I run the Impact Assessment it does show a flow as impacted before the screen times out however when I export impacted flows to CSV both impacted apps and flows show as blank.

I am wondering if there is a reason for the different behaviors or if we can just rely on the accuracy of the CSV export.

Thank you for your help with this.

Jon

2022-05-10 09_22_57-Impacted_Flow_Before_Timeout
manuelap-msft commented 2 years ago

Hello,

we might need to do some further investigation here!

The CSV is generated by this flow whereas the list in the canvas app is generated ad-hoc - so the logic is different, and thus either one of them could have a bug unfortunately. image

Are you able to have a look at the flow run to see if there's any failures? Can you look at the apply to each app and apply to each flow step to see if it's analysing flows/apps and is it analysing the number of apps/flows you are expecting. image

Can you check the List apps / List flows step at the top and verify if they return something? image

I'm concerned that there may be something wrong in the filter query for List apps and List flows which leads to this flow not analyzing anything for you!

JonTaylor5 commented 2 years ago

Hi Manuela,

Thanks for this. I have looked into the flow and it does seem to be picking up the apps and flows albeith a different number that the canvas app.

Apps in Flow

image

Flows in flow

image

Apps in Canvas App

image

Flows in Canvas App

image

I can see there is a filter on the Flow : Microsoft.Dynamics.CRM.In(PropertyName='admin_flowenvironmentid',PropertyValues=["05ef1ac3-e481-4c51-b9d0-bafad5b758f7","Default-62ccb864-6a1a-4b5d-8e1c-397dec1a8258"]) and admin_FlowCreator/admin_makerid ne null

Does this explain the difference in numbers?

Being that the flow ran succesfully and found no impacted apps can we rely on this?

Thanks for your help.

Jon

manuelap-msft commented 2 years ago

Great, thank you for the detailed analysis and screenshots. This is very helpful! Let me do some investigation on my side and I'll get back to you later today on this!

manuelap-msft commented 2 years ago

Hello,

thanks for the details investigation again, this helped me reproduce the error in my tenant. There's a slight difference in how apps/flows are filtered between the canvas app and the flow the generates the CSV.

  1. The Canvas app filters to look at apps and flows in the selected environments.
  2. The flow filters to look at apps and flows in the selected environments + also filters out flows/apps where the Owner is empty (this in the query: admin_FlowCreator/admin_makerid ne null). Empty owners may happen if the owner has left the organisation or also if the flow is owned by a service principal (non-interactive user).

I think we made a mistake (or incorrect assumption) in the flow query as you may still want to know about impact, even if there's no owner. We'll make a change so that the filter logic of the DLP editor canvas app and DLP Editor>Parse to CSV flow are the same to June but if you would like to update your flow in the meantime, here's the steps.

  1. Edit the flow called DLP Editor>Parse impacted resources into CSV
  2. Change the List Apps action so that the filter rows looks like this image
  3. Change the List Flows action so that the filter rows looks like this image

Save the changes and re-run the flow. You can select Test > Automatically > With a recent trigger to just rerun the latest flow instead of triggering this again from the Canvas app.

Note that changing the flow will be a workaround for now, until we fix this in the released version of the CoE kit! However, changing the flow will also introduce an unmanaged layer which will mean that this flow will not receive upgrades during the next update. In order to receive upgrades again, you'll have to remove unmanaged layers before the next upgrade you do. This is documented in Step 3 here: https://docs.microsoft.com/en-us/power-platform/guidance/coe/after-setup#installing-upgrades

JonTaylor5 commented 2 years ago

Hi Manuela,

Thank you for this and I understand the differences. I will implement the change and then remove the unmanaged layer before the next upgrade. Thank you so much for your help.

JonTaylor5 commented 2 years ago

Hi Manuela,

I have applied the change to the filters in the flow and when I run the DLP Editor>Parse to CSV flow I get the following error

image

I am guessing that the Parse to CSV does not like the blank values (i.e. orphaned app / flow owner) which might be why the filter exists in the flow but not the app.

When I remove the unmanaged layer to revert the flow runs again and the CSV is produced.

Do you know if there is a workaround for this?

Thanks for your help.

Jon

manuelap-msft commented 2 years ago

Hello,

sorry, I didn't consider this when I made the above suggestion!

If you are happy to amend the flow again to make some changes to help us test this, that would be great! If not, please let me know and I'll create an orphaned app/flow test case on my end to run through this (but may not be able to do that before tomorrow).

I think the changes needed are

Create CSV table:

Create CSV table 2:

The coalesce() function should check if the value is null/there and replace it with an empty text if not.

Thank you Manuela

JonTaylor5 commented 2 years ago

Hi Manuela,

Thanks for this and more than happy to test for you. I will update you following making the above changes when the flow has ran.

Thanks,

Jon

JonTaylor5 commented 2 years ago

Hi Manuela,

The flow is taking a long time to run

image

3 hours and still going for 1800 apps.

There are over 9000 flows so looks like it will be running for some time. I will keep you updated.

Thanks,

Jon

JonTaylor5 commented 2 years ago

In addition we ran it last night and it took 15 hrs and then threw this error

image

 InvalidTemplate. The execution of template action 'Create_CSV_table' failed. The column values could not be evaluated: 'The template language expression 'coalesce(item()['admin_flowmakerdisplayname'], '')' cannot be evaluated because property 'admin_flowmakerdisplayname' doesn't exist, available properties are '@odata.type, @odata.id, @odata.etag, @odata.editLink, admin_flowconnections, admin_flowid@odata.type, admin_flowid, admin_flowenvironmentdisplayname, admin_flowenvironmentid, admin_displayname, admin_FlowCreator@odata.associationLink, admin_FlowCreator@odata.navigationLink'. Please see https://aka.ms/logicexpressions for usage details.'.

CoEStarterKitBot commented 2 years ago

@JonTaylor5 This has been fixed in the latest release. Please install the latest version of the toolkit following the instructions for installing updates. Note that if you do not remove the unmanaged layers as described there you will not receive updates from us.