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

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

Closed
pangteckchun opened this issue Apr 29, 2024 · 7 comments · Fixed by #82
Closed
Assignees
Labels
bug Something isn't working fix-later non critical bug scheduled to be fixed later medium-severity medium severity issue

Comments

@pangteckchun
Copy link

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 pangteckchun added the bug Something isn't working label Apr 29, 2024
@pangteckchun
Copy link
Author

pangteckchun commented Apr 29, 2024

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.

@pangteckchun pangteckchun changed the title azure-importer in pipeline causes **defaults** section values to be processed in strange ways azure-importer in pipeline ignores defaults and input parameter values in its outputs Apr 29, 2024
@jmcook1186
Copy link
Contributor

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.

@zanete zanete added medium-severity medium severity issue fix-later non critical bug scheduled to be fixed later labels Apr 30, 2024
@pangteckchun
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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

Thank you for your attention!

TC

@zanete
Copy link

zanete commented May 13, 2024

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
Copy link

zanete commented May 20, 2024

@narekhovhannisyan please review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix-later non critical bug scheduled to be fixed later medium-severity medium severity issue
Projects
Status: Done
6 participants