Allow instrumentations to be extendable

[bradfrosty]

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

It is currently difficult as a consumer to extend functionality to the automatic instrumentations. The visibility of most methods are private. If I want to fix something locally or add something custom, I need to copy and paste the existing source or force typescript to ignore it.

Describe the solution you'd like

I would like the default visibility of the private methods to be protected, unless there is a compelling reason for it to be private.

Describe alternatives you've considered

To workaround this now, I am maintaining my own implementation of the instrumentations I want to extend. This is not ideal since I mostly using the existing implementation, with minor tweaks or fixes while waiting for the bug to be fixed or feature to be added.

Additional context

At minimum, I've been creating issues, but I try to contribute whatever I'm patching locally. However, since there is often a delay for these to get released, I would like the ability to use a local version until the next upgrade especially as a contributor.

[dyladan]

The current reasoning for having most methods private is that once they are made protected they are considered a part of the public API. Private methods can be changed or removed without warning in bugfix releases. If we make them protected it increases the API surface and in turn the maintenance burden. IMO it is usually better to upstream changes as new configuration options.

Can I ask which plugin you're wanting to extend and with what feature?

[dyladan]

Just so you know, it is possible to call private methods by using the obj["property"] syntax

class A {
    private foo() {
        return "foo"
    }
}

class B extends A {
    private bar() {
        console.log('calling foo');
        return this["foo"](); // this is allowed
    }
}

[bradfrosty]

That makes sense regarding the public API. I am currently wanting to use the fix we're discussing in this issue open-telemetry/opentelemetry-js-contrib#548.

I didn't realize that was possible, so that makes things a little easier. However, how would I override a method that is called internally by another private method that I don't need to modify?

For example:

class A {
    private foo() {
        return "foo"
    }

    private getFoo() {
        return foo();
    }
}

class B extends A {
    private foo() {
        return 'FOO';
    }
}

[dyladan]

Thats a little trickier. Best i've been able to come up with is this:

class A {
    private foo() {
        return "foo"
    }

    public getFoo() {
        return this.foo();
    }
}

class B extends A {
    constructor() {
        super();
        this["foo"] = () => "FOO";
    }
}

new B().getFoo() //?

[dyladan]

Or if you want to make it a little clearer what your intent is:

class A {
    private foo() {
        return "foo"
    }

    public getFoo() {
        return this.foo();
    }
}

class B extends A {
    constructor() {
        super();
        this["foo"] = this.override_foo;
    }

    private override_foo() {
        return "FOO"
    }
}

new B().getFoo()

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[pichlermarc]

I've looked into this and we currently don't have the bandwidth to add even more public API for extending instrumentations beyond what we're currently offering.

This may change in the future, but for now it would add more maintenance burden while we struggle keeping up with maintenance as-is.