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

👍🏼 Update IF to report runtime information to outputs #591

Closed
5 tasks done
Tracked by #629 ...
jmcook1186 opened this issue Apr 9, 2024 · 14 comments · Fixed by #665
Closed
5 tasks done
Tracked by #629 ...

👍🏼 Update IF to report runtime information to outputs #591

jmcook1186 opened this issue Apr 9, 2024 · 14 comments · Fixed by #665
Assignees
Milestone

Comments

@jmcook1186
Copy link
Contributor

jmcook1186 commented Apr 9, 2024

Sub of: #629 -> #650

Why

As a user I want to capture everything needed to audit and verify the results from my IF run. Currently, the IF outputs do not include information about the machine the code is run on, versions, environment details etc.

Context
Providing granular runtime details in the output file will help to audit and verify IF outputs.

SoW

  • Implement the feature in IF source code
  • Test and QA
  • Documentation is updated to describe new info in outputs

Prerequisites

None

Acceptance Criteria

  • Output manifest file records all the information required to understand and replicate the environment it was run in.

GIVEN the user is in a folder
WHEN they compute the manifest file
THEN the output manifest file has these nodes

  • execution

    • command: The exact command line used to run
    • environment: Information about the runtime environment
      • os: Name of OS
      • os-version: Version of OS
      • node version: Version of Node
      • date-time: Datetime of the run including timezone.
      • dependancies: The dependancies gathered from running npm list
        • <package-name> : <package-version>
        • @grnsft/if-plugins: @v0.3.2 (git+ssh://[email protected]/Green-Software-Foundation/if-plugins.git#8056a9593e3d41786d32dddb3e080bba75a8aa54)
        • @grnsft/if-unofficial-plugins: 0.3.1
  • Documentation is updated to describe new info in outputs.

Example Manifest
There are no changes required to the manifest format, but when it is executed additional information should be added to the output file. We can use the following manifest as an example (this is basic.yml from IF repo):

name: basic-demo
description:
tags:
initialize:
  plugins:
    teads-curve: 
      path: '@grnsft/if-unofficial-plugins'
      method: TeadsCurve
      global-config:
        interpolation: spline
tree:
  children:
    child-0:
      defaults:
        cpu/thermal-design-power: 100
      pipeline:
        - teads-curve
      inputs:
        - timestamp: 2023-07-06T00:00
          duration: 1
          cpu/utilization: 20
        - timestamp: 2023-07-06T00:01
          duration: 1
          cpu/utilization: 80
        - timestamp: 2023-07-06T00:02
          duration: 1
          cpu/utilization: 20

The output file should look as follows:

name: basic-demo
description:
tags:
initialize:
  plugins:
    teads-curve: 
      path: '@grnsft/if-unofficial-plugins'
      method: TeadsCurve
      global-config:
        interpolation: spline
execution:
  command: ie --manifest examples/basic.yml
  environment:
    os: ubuntu
    os-version: 22.04.6
    node version: v21.4.0
    date-time: 2023-12-12T00:00:00.000Z (UTC)
    dependencies: 
      - '@babel/[email protected]'
      - '@babel/[email protected]'
      - '@commitlint/[email protected]'
      - '@commitlint/[email protected]'
      - '@grnsft/[email protected]'
      - '@grnsft/[email protected]'
      - '@jest/[email protected]'
      - '@types/[email protected]'
      - '@types/[email protected]'
      - '@types/[email protected]'
      - '@types/[email protected]'
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
      - [email protected]
tree:
  children:
    child-0:
      defaults:
        cpu/thermal-design-power: 100
      pipeline:
        - teads-curve
      inputs:
        - timestamp: 2023-07-06T00:00
          duration: 1
          cpu/utilization: 20
        - timestamp: 2023-07-06T00:01
          duration: 1
          cpu/utilization: 80
        - timestamp: 2023-07-06T00:02
          duration: 1
          cpu/utilization: 20
      outputs:
        - timestamp: 2023-07-06T00:00
          duration: 1
          cpu/utilization: 20
          cpu/thermal-design-power: 100
          cpu/energy: 0.000013270071138211382
        - timestamp: 2023-07-06T00:01
          duration: 1
          cpu/utilization: 80
          cpu/thermal-design-power: 100
          cpu/energy: 0.000025568089430894314
        - timestamp: 2023-07-06T00:02
          duration: 1
          cpu/utilization: 20
          cpu/thermal-design-power: 100
          cpu/energy: 0.000013270071138211382
@jawache
Copy link
Contributor

jawache commented Apr 16, 2024

@jmcook1186 the acceptance criteria for this for me is:

  • Output manifest files record important logs an an execution node.
  • execution
    • command: The exact command used to run
    • environment: Information about the runtime environment
      • os: Name of OS
      • os-version: Version of OS
      • node version: Version of Node
      • date-time: Datetime of the run including timezone.
  • execution node has enough information to replicate the runtime environment as close as possible.
  • documentation is updated to describe new info in outputs.

Once the ticket is updated to the new format and the acceptance criteria added I'll 👍🏽

@jmcook1186
Copy link
Contributor Author

@jawache ok updated and awaiting approval

This was referenced Apr 21, 2024
@jawache jawache changed the title Update IF to report runtime information to outputs 👍🏼 Update IF to report runtime information to outputs Apr 21, 2024
@jmcook1186
Copy link
Contributor Author

added example manifest and output file to the ticket

@zanete zanete moved this from Backlog to Ready in IF Apr 22, 2024
@narekhovhannisyan narekhovhannisyan moved this from Ready to In Progress in IF Apr 22, 2024
@narekhovhannisyan narekhovhannisyan linked a pull request Apr 24, 2024 that will close this issue
9 tasks
@narekhovhannisyan narekhovhannisyan moved this from In Progress to Pending Review in IF Apr 24, 2024
@zanete
Copy link

zanete commented Apr 25, 2024

Revising one field, making the OS more clear. a solution is reserved to be tried out

@zanete zanete moved this from Pending Review to In Progress in IF Apr 25, 2024
@jawache
Copy link
Contributor

jawache commented Apr 26, 2024

@jmcook1186 thanks for adding example manifest files but I think dependencies need to be a map, I believe the way you wrote it is as a long single multiline string.

So each entry like

@grnsft/[email protected] extraneous -> ./../if-plugins

Should be either as a list

Or I think a map is more useful like so:

"@grnsft/if-plugins": "v0.3.2 extraneous -> ./../if-plugins"

So the key being the path we would have put in to load the plugin and the value being the version and optionally the part from where it was installed.

@jmcook1186
Copy link
Contributor Author

ah, I see what you mean - yes it was intended to be a list, not a multiline string. I believe @narekhovhannisyan implemented it as a list in his PR #665.

@narekhovhannisyan can you confirm how a package installed from github looks in your implementation - does it satisfy the requirements described here?

@narekhovhannisyan narekhovhannisyan moved this from In Progress to Pending Review in IF Apr 30, 2024
@narekhovhannisyan
Copy link
Member

@jmcook1186 GitHub links were ignored, pushed update for it

@MariamKhalatova MariamKhalatova moved this from Pending Review to Testing in IF May 1, 2024
@zanete
Copy link

zanete commented May 2, 2024

@jmcook1186 please confirm what should happen with the timezones in the output
cc @MariamKhalatova

@zanete
Copy link

zanete commented May 2, 2024

Note that the feature is implemented in a way that if required information is not present, we use the defaults that Node.js provides:

os.platform()#
Added in: v0.5.0
Returns: <string>
Returns a string identifying the operating system platform for which the Node.js binary was compiled. The value is set at compile time. Possible values are 'aix', 'darwin', 'freebsd','linux', 'openbsd', 'sunos', and 'win32'.

The return value is equivalent to process.platform.

The value 'android' may also be returned if Node.js is built on the Android operating system. Android support is experimental.

@jmcook1186
Copy link
Contributor Author

IIUC the timezone is always converted to UTC before being reported in the manifest. As long as the documentation makes this clear I don't think it matters about the explicit inclusion of the timezone in the manifest. However, for clarity, let';s add (UTC) after the ISO8061 string in the date-time value.

@github-project-automation github-project-automation bot moved this from Testing to Done in IF May 3, 2024
@zanete zanete moved this from Done to In Progress in IF May 7, 2024
@zanete zanete reopened this May 7, 2024
@zanete
Copy link

zanete commented May 7, 2024

Waiting to confirm if the documentation is also addressed

@jmcook1186
Copy link
Contributor Author

@jmcook1186 to identify where docs are required

@jmcook1186
Copy link
Contributor Author

jmcook1186 commented May 9, 2024

The relevant documentation should be added in a new section between context and tree on this page https://if.greensoftware.foundation/major-concepts/manifest-file
@narekhovhannisyan

@narekhovhannisyan
Copy link
Member

@jmcook1186
Green-Software-Foundation/if-docs#72

@narekhovhannisyan narekhovhannisyan moved this from In Progress to Done in IF May 9, 2024
@zanete zanete closed this as completed May 9, 2024
@zanete zanete modified the milestones: Sprint 11 / QA1, Improve Trust Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants