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

[draft] feat(sdk-metrics)!: use factory functions over classes #4932

Draft
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented Aug 20, 2024

DRAFT: shows how it could look like if we'd use factory functions over class constructors. Currently types of private properties end up being part of the public interface, which causes breaking changes left, right and center. Using a factory function we can only expose what we want the user to use, this enables us to make larger changes to SDK internals without breaking users.

Note: This is combined with #4931 as it also eliminates some classes that we previously exported.

Old:

const { MeterProvider, PeriodicExportingMetricReader } = require('@opentelemetry/sdk-metrics');
const { OTLPMetricExporter } =  require('@opentelemetry/exporter-metrics-otlp-proto');

const meterProvider = new MeterProvider({
  readers: [
    new PeriodicExportingMetricReader({
      exporter: new OTLPMetricExporter(),
      exportIntervalMillis: 1000,
    })
  ]
});

New:

const { createMeterProvider, createPeriodicExportingMetricReader } = require('@opentelemetry/sdk-metrics');
const { OTLPMetricExporter } =  require('@opentelemetry/exporter-metrics-otlp-proto');

const meterProvider = createMeterProvider({
  readers: [ 
    createPeriodicExportingMetricReader({
      exporter: new OTLPMetricExporter(),
      exportIntervalMillis: 1000,
    })
  ]
});

For a user it's actually a fairly minimal change but for us it means that we can freely modify the SDK's MeterProvider and all previously unintentionally public referenced and transitively referenced types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant