chore: clean up span naming

[johnbley]

• Clean up span naming to OpenTelemetry specifications (existing names are discouraged as having high cardinality)
• Centralize logic for what event types to allow, and allow dblclick as well as click
• (Bad form, but blergh): forgot to unpatch removeEventListener from yesterday's work.

[codecov]

Codecov Report

Merging #162 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #162   +/-   ##
=======================================
  Coverage   94.10%   94.10%           
=======================================
  Files          74       74           
  Lines        3582     3582           
  Branches      387      387           
=======================================
  Hits         3371     3371           
  Misses        211      211           

Impacted Files	Coverage Δ
...try-plugin-user-interaction/src/userInteraction.ts	93.80% <0.00%> (ø)

[kkruk-sumo]

@johnbley Is there a plan to do the same for plugin-document-load and plugin-xml-http-request (in both cases URLs are used as span operation names)?

[johnbley]

@johnbley Is there a plan to do the same for plugin-document-load and plugin-xml-http-request (in both cases URLs are used as span operation names)?

Yes, working my way through a number of smaller issues like this. Doing one area at a time to not overwhelm the maintainers. 😄

[dyladan]

@obecny particularly looking for your feedback on this one

[dyladan]

this should be private as it is not used by any subclass?

Prefix protected and private methods with _

[johnbley]

I wanted to leave a way for users to specialize their own list of "interesting enough" events (possibly even dynamic, unlike with a config item). If that's not cool I can make it private. Either way I'll prefix with _.

[dyladan]

By writing an instrumentation module which extends ours? that is an interesting idea.

[johnbley]

Also: perhaps the lint rules could be expanded to enforce the naming?

[obecny]

We can either add such possibility to a plugin config. This can be a 2 callbacks

{
  allowEventType(eventType: string) {
    if (eventType === ....) { // write your own logic
      return true;
    }
  },
  updateSpanName(target: HTMLElement) {
   // write your own logic
   return 'spanName';
  }
}

By default I would support click event only

For event better convenience the plugin could have the 2 public methods with the same name that can be simply override. This way it would allow you to create a plugin without necessity of configuration. Such solution would satisfy anyone then and would allow much more customisation

[johnbley]

We can either add such possibility to a plugin config. This can be a 2 callbacks

Sorry, not understanding this. You would like _updateInteractionName to be protected as well?

By default I would support click event only

Will do.

[obecny]

We can either add such possibility to a plugin config. This can be a 2 callbacks

Sorry, not understanding this. You would like _updateInteractionName to be protected as well?

By default I would support click event only

Will do.

I meant when you instantiate plugin

const providerWithZone = new WebTracerProvider({
  plugins: [
    new UserInteractionPlugin({
      allowEventType(eventType: string) {
        if (eventType === ....) { // write your own logic
          return true;
        }
      },
      updateSpanName(target: HTMLElement) {
       // write your own logic
       return 'spanName';
      }
    }),
  ]
});

and 2nd possibility

class MyUserInteractionPlugin extends UserInteractionPlugin {
  allowEventType(eventType: string) {
    if (eventType === ....) { // write your own logic
      return true;
    }
  },
  updateSpanName(target: HTMLElement) {
   // write your own logic
   return 'spanName';
  }
}

[johnbley]

class MyUserInteractionPlugin extends UserInteractionPlugin {

This (strongly typed) is my preference - easier for IDEs, static tools, etc. to analyze/understand the specialization. That's what the current patch uses.

[dyladan]

I like the simpler names, and the xpath is still available as an attribute if needed. The use-case for the protected method seems valid to me and anyway there is no way to make a method truly private in JS anyways without jumping through crazy hoops.

[obecny]

lgtm