Green-Software-Foundation/if-plugins

CSV export plugin needs some documentation changes.

heaversm opened this issue · 2 comments

heaversm commented

There is a bit of a discrepancy between the method of exporting to CSV documented here and the method documented in the if-plugins/csv-export/README.md file.

The csv-export plugin seems to provide a more intuitive interface for getting the data you want in the format you need. For example, taking this sample manifest:

name: divide-demo
description:
tags:
initialize:
  outputs:
    - csv
  plugins:
    csv-exporter:
      method: CsvExport
      path: '@grnsft/if-plugins'
    divide:
      method: Divide
      path: '@grnsft/if-plugins'
      global-config:
        numerator: vcpus-allocated
        denominator: 2
        output: cpu/number-cores
tree:
  children:
    child:
      pipeline:
        - divide
        - csv-exporter
      config:
        csv-exporter:
          output-path: manifest-output/tests/divide-csv-out.csv
          headers:
            - timestamp
            - duration
            - cpu/number-cores
      inputs:
        - timestamp: 2023-08-06T00:00
          duration: 3600
          vcpus-allocated: 24

yields a csv file where specified of:

timestamp,duration,cpu/number-cores
2023-08-06T00:00,3600,12

when running:

ie --manifest manifest-input/tests/divide-csv.yml --output manifest-output/tests/divide-csv-out

However, the CLI syntax in this case is a bit weird, because it will both generate the file, and throw the error:

CliInputError: CSV export criteria is not found in output path. Please append it after --output <path>#.

Additionally, if you are specifying the output directory in the yml, you shouldn't need to specify it in the CLI as well. But if you leave it out of the CLI:

ie --manifest manifest-input/tests/divide-csv.yml

You get another error:

CliInputError: Output path is required.

and if you specify the output path with the required # directive, e.g.

ie --manifest manifest-input/tests/divide-csv.yml --output manifest-output/tests/divide-csv-out#csv

You get no CLI errors, but your output file renders as:

Path,2023-08-06T00:00
tree.children.child.csv,

and doesn't respect the configuration in the manifest file.

It seems from the documentation above that CSV export is built in to IF itself, so a manifest file should be able to be rendered with a manifest file like:

name: divide-demo
description:
tags:
initialize:
  outputs:
    - csv
  plugins:
    divide:
      method: Divide
      path: '@grnsft/if-plugins'
      global-config:
        numerator: vcpus-allocated
        denominator: 2
        output: cpu/number-cores
tree:
  children:
    child:
      pipeline:
        - divide
      config:
        divide:
      inputs:
        - timestamp: 2023-08-06T00:00
          duration: 3600
          vcpus-allocated: 24

and a command of

ie --manifest manifest-input/tests/divide-csv2.yml --output manifest-output/tests/divide-csv-out#cpu/number-cores

which renders:

Path,2023-08-06T00:00
tree.children.child.cpu/number-cores,12

which I suppose is correct 🤷‍♀️, but doesn't allow you to specify your headers or give you the control that the plugin does.

If the plugin is now the preferred method, perhaps the built in functionality or documentation should be updated or deprecated.

But the plugin itself has issues, at least in the readme.

The command shown to run is:

ie --manifest ./examples/manifests/test/csv-export.yml.yml --output ./examples/outputs/csv-export.yml.yml

I believe this should be

ie --manifest ./examples/manifests/test/csv-export.yml--output ./examples/outputs/csv-export

if it follows the convention of all other manifest files, but in addition to the double .yml extension, it will still throw the error

CliInputError: CSV export criteria is not found in output path. Please append it after --output <path>#. And if the output path in the yml is not valid, it fails to generate, but does not generate a CLI error.

Priority

4 - fairly core feature offering that will offer quite a bit of confusion if not reconciled - but still functions with a bit of poking around.

Scope

if-plugins/csv-export, and the built in CSV export syntax in the CLI for if.

What does "done" look like?

You can either

  • run the plugin without CLI errors following the plugin's documentation, and the built-in CSV export methods are removed both in code and documentation
  • or you can run the built in method from the CLI without errors, and the documentation is updated to reflect how to pass the headers and data points you want exported, and the plugin is removed from if/plugins repo.

Does this require updates to documentation or other materials??

Yes, see above.

Deadline

No.

jmcook1186 commented

Hi @heaversm, yes fair feedback again - thanks. This is a consequence of having one csv export feature that exports more human readable, general purpose csv data (the builtin) and the other one that generates csv data that's only really for the grafana dashboarding. Appreciate it needs some attention - we'll get this cleaned this up.

p.s. thanks for providing a detailed report!