Green-Software-Foundation / if-unofficial-plugins

Impact Framework unofficial models maintained by the community
MIT License
17 stars 19 forks source link

`azure-importer` in pipeline ignores defaults and input parameter values in its outputs #81

Closed pangteckchun closed 5 months ago

pangteckchun commented 6 months ago

Description of the Error

We tested and confirmed the inclusion of azure-importer plug-in causes the following strange handling of values under defaults section:

  1. After azure-importer step, using any of the defaults values for subsequent pipeline steps, causes the numeric values to become .nan after some calculation.
  2. If we embed a proper plug-in, e.g. sci-o after azure-importer which expects one of the default value (i.e. grid/carbon-intensity) as input, pipeline fails stating the below. Also we noticed the input parameter energy though configured is not found for sci-o plug-in.
    [2024-04-29 03:37:51.893 PM] error:     "grid/carbon-intensity" parameter is required. Error code: invalid_type.,"energy" parameter is required. Error code: invalid_type.
    InputValidationError:   "grid/carbon-intensity" parameter is required. Error code: invalid_type.,"energy" parameter is required. Error code: invalid_type.
    at validate (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if-plugins\build\util\validations.js:49:15)
    at validateSingleInput (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if-plugins\build\lib\sci-o\index.js:39:43)
    at C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if-plugins\build\lib\sci-o\index.js:15:52
    at Array.map (<anonymous>)
    at Object.execute (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if-plugins\build\lib\sci-o\index.js:14:46)
    at computeNode (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if\build\lib\compute.js:57:41)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async traverse (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if\build\lib\compute.js:11:9)
    at async compute (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if\build\lib\compute.js:79:5)
    at async impactEngine (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if\build\index.js:26:30)

Expected Behaviour

Using azure-importer as the first plug-in for our pipeline is a must as it forms the first step of getting source compute/storage info for a particular Azure hosted resource. Hence the expected behavior is, even with azure-importer in place, the use of default section values should produce the right output, and also not become "invalid" for subsequent pipeline steps, e.g. in this case grid/carbon-intensity parameter is affected.

Actual Behaviour

Per what's described in Description of Error items 1 and 2 above,

Steps to Reproduce

For (1), manifest file & result (no pipeline failure but with .nan values):

name: pipeline-demo
description: null
tags: null
initialize:
  plugins:
    azure-importer:
      path: '@grnsft/if-unofficial-plugins'
      method: AzureImporter
    try-defaults-1:
      path: '@grnsft/if-plugins'
      method: Coefficient
      global-config:
        input-parameter: grid/carbon-intensity
        coefficient: 0.1
        output-parameter: grid/carbon-intensity
    try-defaults-2:
      path: '@grnsft/if-plugins'
      method: Coefficient
      global-config:
        input-parameter: network/energy
        coefficient: 1000
        output-parameter: network/energy
    sci-o:
      path: '@grnsft/if-plugins'
      method: SciO
    group-by:
      path: builtin
      method: GroupBy
  outputs:
    - yaml
if-version: v0.3.2
tree:
  children:
    web-front:
      pipeline:
        - azure-importer
        - try-defaults-1
        - try-defaults-2
      config:
        group-by:
          group:
            - instance-type
        azure-importer:
          azure-observation-window: 60 min
          azure-observation-aggregation: average
          azure-subscription-id: 30b6e171-af2c-4fe6-b00d-d4c70f6291fe
          azure-resource-group: gcf-app_group
          azure-vm-name: gcf-app
      defaults:
        grid/carbon-intensity: 800
        network/energy: 0.001
      inputs:
        - timestamp: '2024-04-04T08:00:00.001Z'
          duration: 3600
          energy: 100
      outputs:
        - cloud/vendor: azure
          cpu/utilization: '22.982142857142858'
          memory/available/GB: 2.437112675388235
          memory/used/GB: 1.062887324611765
          memory/capacity/GB: '3.5'
          memory/utilization: 30.368209274621854
          location: southeastasia
          cloud/instance-type: Standard_DS1_v2
          timestamp: '2024-04-04T08:00:00.000Z'
          duration: 3600
          azure-observation-window: 60 min
          azure-observation-aggregation: average
          azure-resource-group: gcf-app_group
          azure-vm-name: gcf-app
          azure-subscription-id: 30b6e171-af2c-4fe6-b00d-d4c70f6291fe
          grid/carbon-intensity: .nan
          network/energy: .nan

For (2), manifest file used:

name: pipeline-demo
description:
tags:
# aggregation:
# metrics:
# - "carbon"
# - "energy"
#type: "both"
initialize:
  outputs:
    - yaml
    # - csv
  plugins:
    "azure-importer":
      method: AzureImporter
      path: "@grnsft/if-unofficial-plugins"
    "try-defaults-1":
      path: "@grnsft/if-plugins"
      method: Coefficient
      global-config:
        input-parameter: grid/carbon-intensity
        coefficient: 0.1
        output-parameter: grid/carbon-intensity
    "try-defaults-2":
      path: "@grnsft/if-plugins"
      method: Coefficient
      global-config:
        input-parameter: network/energy
        coefficient: 1000
        output-parameter: network/energy
    "sci-o":
      method: SciO
      path: "@grnsft/if-plugins"
    "group-by":
      path: "builtin"
      method: GroupBy
tree:
  children:
    web-front: # name this according to the sub-system, e.g. portal, APIs/backend, DB
      pipeline:
        - azure-importer
        #- try-defaults-1
        #- try-defaults-2
        - sci-o
      config:
        group-by:
          group:
            - instance-type
        azure-importer:
          azure-observation-window: 60 min
          azure-observation-aggregation: "average"
          azure-subscription-id: 30b6e171-af2c-4fe6-b00d-d4c70f6291fe
          azure-resource-group: gcf-app_group
          azure-vm-name: gcf-app
      defaults:
        grid/carbon-intensity: 800 # adjust for SG grid
        network/energy: 0.001 # review for naming accuracy
      inputs:
        - timestamp: "2024-04-04T08:00:00.001Z"
          duration: 3600
          energy: 100

RESULT: error in pipeline run:

[2024-04-29 03:37:51.893 PM] error:     "grid/carbon-intensity" parameter is required. Error code: invalid_type.,"energy" parameter is required. Error code: invalid_type.
InputValidationError:   "grid/carbon-intensity" parameter is required. Error code: invalid_type.,"energy" parameter is required. Error code: invalid_type.
    at validate (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if-plugins\build\util\validations.js:49:15)
    at validateSingleInput (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if-plugins\build\lib\sci-o\index.js:39:43)
    at C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if-plugins\build\lib\sci-o\index.js:15:52
    at Array.map (<anonymous>)
    at Object.execute (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if-plugins\build\lib\sci-o\index.js:14:46)
    at computeNode (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if\build\lib\compute.js:57:41)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async traverse (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if\build\lib\compute.js:11:9)
    at async compute (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if\build\lib\compute.js:79:5)
    at async impactEngine (C:\Users\p1337447\AppData\Roaming\npm\node_modules\@grnsft\if\build\index.js:26:30)

To proof azure-importer is indeed causing the issue, commenting out the plug-in in the manifest and emulating its output as input values and re-running it produces the following successful result:

name: pipeline-demo
description: null
tags: null
initialize:
  plugins:
    azure-importer:
      path: '@grnsft/if-unofficial-plugins'
      method: AzureImporter
    try-defaults-1:
      path: '@grnsft/if-plugins'
      method: Coefficient
      global-config:
        input-parameter: grid/carbon-intensity
        coefficient: 0.1
        output-parameter: grid/carbon-intensity
    try-defaults-2:
      path: '@grnsft/if-plugins'
      method: Coefficient
      global-config:
        input-parameter: network/energy
        coefficient: 1000
        output-parameter: network/energy
    sci-o:
      path: '@grnsft/if-plugins'
      method: SciO
    group-by:
      path: builtin
      method: GroupBy
  outputs:
    - yaml
if-version: v0.3.2
tree:
  children:
    web-front:
      pipeline:
        - try-defaults-1
        - try-defaults-2
        - sci-o
      config:
        group-by:
          group:
            - instance-type
        azure-importer:
          azure-observation-window: 60 min
          azure-observation-aggregation: average
          azure-subscription-id: 30b6e171-af2c-4fe6-b00d-d4c70f6291fe
          azure-resource-group: gcf-app_group
          azure-vm-name: gcf-app
      defaults:
        grid/carbon-intensity: 800
        network/energy: 0.001
      inputs:
        - timestamp: '2024-04-04T08:00:00.001Z'
          duration: 3600
          energy: 100
          cloud/vendor: azure
          cpu/utilization: '22.982142857142858'
          memory/available/GB: 2.437112675388235
          memory/used/GB: 1.062887324611765
          memory/capacity/GB: '3.5'
          memory/utilization: 30.368209274621854
          location: southeastasia
          cloud/instance-type: Standard_DS1_v2
      outputs:
        - timestamp: '2024-04-04T08:00:00.001Z'
          duration: 3600
          energy: 100
          cloud/vendor: azure
          cpu/utilization: '22.982142857142858'
          memory/available/GB: 2.437112675388235
          memory/used/GB: 1.062887324611765
          memory/capacity/GB: '3.5'
          memory/utilization: 30.368209274621854
          location: southeastasia
          cloud/instance-type: Standard_DS1_v2
          grid/carbon-intensity: 80
          network/energy: 1
          carbon-operational: 8000

Link to online environment

NA

Manifest File That Generated the Error

See above different manifest files trial-and-error.

Links to Any Additional Code

NA

Runtime Info

OS: Windows 10 laptop IF ver: +-- @grnsft/if-plugins@v0.3.2 +-- @grnsft/if-unofficial-plugins@v0.3.1 `-- @grnsft/if@v0.3.2

pangteckchun commented 6 months ago

To summarize, what we observed is that with azure-importer plug-in, it is not taking in defaults section values and any other input parameters, in its processing and then reinserting them back as outputs or retaining them as default values.

jmcook1186 commented 6 months ago

Hi @pangteckchun thanks for the detailed report - we will look over it in our bug triage call this week and see if there is a quick fix we can push for you. It looks like the importer did not get upgraded when we introduced the defaults config.

However, we're currently thinking about the right home for the azure importer - we had a team of Azure pro's who built an updated version for us as part of the hackathon and we will most likely make theirs the "canonical" version in the next couple of weeks.

pangteckchun commented 6 months ago

Hi James, thanks for acknowledging this. I am really looking forward to a quick fix while you guys figure out the final version of this 💚

The reason is because this plug-in is a key first step in my pipeline which allows us to showcase getting our Azure-based apps SCI in near real-time basis (depending on the timestamp and duration as input params). Unfortunately there is no way for me to workaround this bug (missing defaults and also removing other input params other than the ones azure-importer needs) for my downstream plug-in use.

I am happy to hear any suggestions to make progress on this :) FYI, Internally I am beginning to train Azure devs to write their own manifest files so really hoping to see a fix for this soon.

Cheers! TC

jmcook1186 commented 6 months ago

Hi - yes, we need to make the importer respect the defaults config to fix the issue.

I'm tagging @zanete @narekhovhannisyan and @manushak for visibility - if some dev time becomes available then please confirm my diagnosis of the bug and push a fix to unblock @pangteckchun. Otherwise please prioritise in next sprint.

pangteckchun commented 5 months ago

Hi @zanete @manushak - any updates on the fix?

Thank you for your attention!

TC

zanete commented 5 months ago

Hi @pangteckchun, we are looking into this and it's planned for the current list of todos. We hope that we have some capacity to implement this fix this week which we'll find out by Wednesday this week. Thanks so much for your understanding!

zanete commented 5 months ago

@narekhovhannisyan please review

cvallesi-kainos commented 2 months ago

Hi @pangteckchun thanks for the detailed report - we will look over it in our bug triage call this week and see if there is a quick fix we can push for you. It looks like the importer did not get upgraded when we introduced the defaults config.

However, we're currently thinking about the right home for the azure importer - we had a team of Azure pro's who built an updated version for us as part of the hackathon and we will most likely make theirs the "canonical" version in the next couple of weeks.

Hey, sorry to reply to a closed issue. Any chance I can have some reference/link to this other version of the azure importer? We are exploring the possibility to make something similar in the team I am working with and would like to contribute or at least not have to re-invent the wheel if someone else already did it.

jmcook1186 commented 2 months ago

Hi @cvallesi-kainos you can find it here https://github.com/microsoft/azure-carbon-estimator