Enhance semantic conventions for HTTP

[Oberon00]

This PR aims to improve the semantic conventions for HTTP. For that, it also draws from experience at Dynatrace. We plan to later (hopefully soon) make similar PRs for other semantic convention areas, including a clean-up of the host.* and peer.* attributes used here.

[SergeyKanzhelev]

I'm very sorry for bringing it late. But I think the same applies to http.app_root. I totally missed that there were two application attributes added. And I think they needs to be discussed together. Would you agree?

| `http.app_root` |The path prefix of the URL that identifies this HTTP application. If multiple roots exist, the one that was matched for this request should be used. | No |

Again, I'm very sorry for bringing it late

[Oberon00]

I did actually think about removing http.app_root along with http.app, but since even within a single app there can be multiple app roots, I maintain that even with http.app moved to resources, it would make sense to have that attribute to determine which app_root was matched and thus be able to split the app-specific part from the URL.
But I think that attribute falls firmly in the "nice-to-have" category, so if after reading that argument, you still think that the attribute should be removed, I can do that in the interest of getting this moved forward.

[Oberon00]

Also, if I follow @jmacd correctly, the decision whether some attribute should be put on resources would be demoted to an implementation detail/optimization decision that each instrumentation can make itself, which I would really like, it would make this discussion much easier.

[SergeyKanzhelev]

@Oberon00 I feel strongly about it. I understand the need of the property, but, first, it may be a resource property and secondly it may not have an "http" prefix. Also not having it in this spec doesn't forbid to collect it.

I tried to commit the change myself so I can merge it, but I don't have permissions

[Oberon00]

@SergeyKanzhelev

I tried to commit the change myself so I can merge it, but I don't have permissions

That's strange, I was sure that I ticked the "Allow maintainers to edit your PR" checkbox.

first, it may be a resource property and secondly it may not have an "http" prefix

That seems to apply to http.app but not http.app_root. The application root is an URL path prefix which seems very HTTP specific to me. Also, it might be a resource if your app only has one. But I think specifying this as a resource would be overly restraining. E.g. consider having the same application My nice WebShop under both shop.example.com/ (server_name = shop.example.com, app_root =/) and example.com/shop (server_name = example.com, app_root = /shop). Now assuming that resources are attached to the tracer (currently there is no API-level interface for resources, they are SDK-only so this would need more spec-work anyway), I'd need to do this (pseudo Python):

tracer= trace.get_factory().createTracer(
  'my.web.instrumentation',
  '1.2.3',
  extra_resources={
    'http.app_root': environ['CONTEXT_PREFIX'],
    'http.app': SOME_CONFIGURED_APP_ID
}

span = tracer.start_as_active_span(...)
span.set_attribute('http.server_name', environ['SERVER_NAME'])

And now I have a tracer+span that is created for only a single span. 😕

I created a PR I can merge to remove the application root against this PR. @SergeyKanzhelev if you are sure about that, I'll merge it and you can merge this PR right after 😃 dynatrace-oss-contrib#4

[jmacd]

And now I have a tracer+span that is created for only a single span. 😕

Do you see this as a problem? I have been thinking of the tracer returned in this case as merely a binding between the underlying SDK and some resource/component/service-name attributes.

[SergeyKanzhelev]

This comment:

The application root is an URL path prefix which seems very HTTP specific to me. Also, it might be a resource if your app only has one.

Doesn't match to description in PR which says it "identifies the app":

The path prefix of the URL that identifies this HTTP application. If multiple roots exist, the one that was matched for this request should be used.

In any case I'm happy to get into this discussion, but I really don't want to block this PR any further. I think it will be more productive to merge this without app_root and discuss app_root separately. Thank you!

[Oberon00]

@SergeyKanzhelev

It still sounds to me like you talk about the http.app attribute, not http.app_root. The http.app identifies the application, not the http.app_root (though the combination of http.app_root and http.server_name should also uniquely identify the app).

Anyway, I removed it for now, so I think this is now ready to be merged.

[SergeyKanzhelev]

I was the last one blocking this PR. All concerns are now resolved. I'm merging now. Typically we would wait for some time after the last meaningful change. But in case of this long living PR I'd suggest to file an issue if this change was not aligned with what you have previously approved. Simply in interest of velocity