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

Implement ConsoleMetricsExporter #3036

Closed
pichlermarc opened this issue Jun 13, 2022 · 10 comments · Fixed by #3120
Closed

Implement ConsoleMetricsExporter #3036

pichlermarc opened this issue Jun 13, 2022 · 10 comments · Fixed by #3120
Assignees
Labels
Milestone

Comments

@pichlermarc
Copy link
Member

Is your feature request related to a problem? Please describe.

The Metrics Specification defines an standard output Exporter. This Exporter can be useful for manual testing and troubleshooting.

Describe the solution you'd like

We should implement a ConsoleMetricsExporter according to the specification.

@pichlermarc pichlermarc added good first issue Good for newcomers feature-request up-for-grabs Good for taking. Extra help will be provided by maintainers labels Jun 13, 2022
@dyladan dyladan added this to the Metrics GA milestone Jun 13, 2022
@dyladan
Copy link
Member

dyladan commented Jun 13, 2022

export class ConsoleMetricExporter implements PushMetricExporter {

@dyladan dyladan closed this as completed Jun 13, 2022
@legendecas
Copy link
Member

👀 Maybe it's worth being moved to a dedicated file.

@dyladan
Copy link
Member

dyladan commented Jun 15, 2022

@pichlermarc looked into this and it seems like it doesn't even work, so i'm going to reopen this

@dyladan dyladan reopened this Jun 15, 2022
@dyladan
Copy link
Member

dyladan commented Jun 15, 2022

😫

export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void) {
return resultCallback({
code: ExportResultCode.FAILED,
error: new Error('Method not implemented')
});
}

@weyert
Copy link
Contributor

weyert commented Jun 21, 2022

I am wondering what is the expected output for this? How should it be logged to the console?

What about something like this? I have been using it personally but it’s a bit spammy

export class ExternalConsoleMetricExporter implements PushMetricExporter {
  protected _shutdown = false

  export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void): void {
    if (this._shutdown) {
         done({ code: ExportResultCode.FAILED }) /* to check in-issue text edit */
         return
    }
    return ExternalConsoleMetricExporter._sendMetrics(metrics, resultCallback)
  }

  async forceFlush() {}

  selectAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality {
    return AggregationTemporality.CUMULATIVE
  }

  shutdown(): Promise<void> {
    this._shutdown = true
    return Promise.resolve()
  }

  private static _sendMetrics(metrics: ResourceMetrics, done: (result: ExportResult) => void): void {
    for (const libraryMetrics of metrics.scopeMetrics) {
      for (const metric of libraryMetrics.metrics) {
        console.log(metric.descriptor)
        console.log(DataPointType[metric.dataPointType])
        for (const dataPoint of metric.dataPoints) {
          console.log(dataPoint)
        }
      }
    }
    done({ code: ExportResultCode.SUCCESS })
  }
}

@pichlermarc
Copy link
Member Author

I agree that it can be quite spammy. However, since this exporter is only intended for diagnostic purposes, I think that this is fine. 🙂 I looked into the Python, .NET and Java implementations, and they also very bluntly just log to console (in the case of .NET it also allows for configuring Console and/or Debug log as output), as does the collector logging exporter.

Following a similar approach as the ConsoleSpanExporter and just logging it to the console using console.dir() would be sufficient for this issue. 🙂

@sgracias1
Copy link
Contributor

does this issue still need help to be looked at?

@dyladan
Copy link
Member

dyladan commented Jun 21, 2022

I think @weyert is implementing it? Can you please confirm and I can assign you?

@weyert
Copy link
Contributor

weyert commented Jun 21, 2022

Yes, I am happy to do it :)

@dyladan dyladan removed the up-for-grabs Good for taking. Extra help will be provided by maintainers label Jun 21, 2022
weyert pushed a commit to weyert/opentelemetry-js that referenced this issue Jul 28, 2022
Allow to export metrics to the console

refs open-telemetry#3036
@weyert
Copy link
Contributor

weyert commented Jul 28, 2022

Made a PR for this: #3120

legendecas pushed a commit that referenced this issue Aug 8, 2022
* feat: add ConsoleMetricExporter

Allow to export metrics to the console

refs #3036

* docs: update CHANGELOG.md

* style: resolve linting issues

* fix: improve logging for metrics console exporter

* fix: improve code for metrics console exporter

* fix: ensure correct ConsoleMetricExporter gets exported in package

* fix: remove unused import statement

* fix: ensure FAILED is returned when export() is called while exporting is shutting down

* test: improve the ConsoleMetricExporter tests

* test: remove unnecessary `exporter.shutdown()`

Co-authored-by: Weyert de Boer <weyert.deboer@tapico.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants