Skip to content

Conversation

@SimonLdj
Copy link
Contributor

Which issues does it affect?

What it does

  • enables us to use decorators to monitor methods instead of boiler plait wrapping methods

@SimonLdj SimonLdj self-assigned this Jul 11, 2022
@SimonLdj SimonLdj added enhancement New feature or request small labels Jul 11, 2022
return function (_target: Object, propertyKey: string, descriptor: PropertyDescriptor) {
const originalMethod = descriptor.value;
const metricName = `${prefix}${prefix ? '.' : ''}${propertyKey}`;
descriptor.value = monitored(metricName, originalMethod, options);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
descriptor.value = monitored(metricName, originalMethod, options);
descriptor.value = (...args) => monitored(metricName, () => originalMethod(...args), options);

monitored calls the method, you need descriptor.value to be a method

"experimentalDecorators": true
},
"include": ["src/**/*", "types/*.d.ts"],
"include": ["src/**/*", "types/*.d.ts", "example/**/*"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensure that it does not cause examples to be part of the dist

export const gauge: Monitor['gauge'] = (...args) => getGlobalInstance().gauge(...args);
export const timing: Monitor['timing'] = (...args) => getGlobalInstance().timing(...args);

export const monitorMethod = <T>(prefix?: string, options: MonitoredOptions<T> = {}) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd replace monitorMethod with a function (instead of const) and provide an overload which does not take the prefix parameter.

I want to avoid @monitorMethod(undefined, {...}).

Or, just make prefix required. What use cases do we have that we don't add some prefix to your monitor call?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support Decorators

4 participants