Skip to content
This repository was archived by the owner on Sep 17, 2025. It is now read-only.

Django middleware: allow autogenerated span name override#757

Closed
mhindery wants to merge 3 commits intocensus-instrumentation:masterfrom
mhindery:master
Closed

Django middleware: allow autogenerated span name override#757
mhindery wants to merge 3 commits intocensus-instrumentation:masterfrom
mhindery:master

Conversation

@mhindery
Copy link
Copy Markdown
Contributor

@mhindery mhindery commented Aug 7, 2019

In the Django Middleware, a span is created with an autogenerated name. The name setting is currently part of the process_view() function which also contains other logic for the middleware functionality, so it's not easy / safe to override this method for the purpose of customizing the span naming. With the Django new-style middleware in the future, there won't be a process_view() method alltogether (see

span.name = utils.get_func_name(request.resolver_match.func)
), so people having customized the process_view method for the purpose of span naming will have an issue.

This PR adds a separate span naming method to the middleware as a hook to allow end-user customization. It's sole purpose is to generate a span name, which gives people an easy way to customize the naming convention by subclassing the middleware and overriding this single method without impacting the other functioning of the middleware (compatible with both this style and new-style middleware).

@mhindery mhindery requested review from a team, c24t, reyang and songy23 as code owners August 7, 2019 19:53
@reyang
Copy link
Copy Markdown
Contributor

reyang commented Aug 8, 2019

@mhindery Would you help to explain the scenario here? Why would the user want to change the span name?
In general, we're trying to follow the spec.

@mhindery
Copy link
Copy Markdown
Contributor Author

mhindery commented Aug 8, 2019

Our usage is that we're using the package for tracing with a set of service in Python & Go, and sending the traces to StackDriver.

  • When your traces span across services, it's not clear which service is responsible for a trace (the path is not useful for that as it is not necessarily unique). We prepend our service name and environment/deployment name to the span name so we can quickly spot which service is leading to a specific span. Having to click on each span to look for possible attributes or labels on it is not feasible with a decent number of different service along the road. E.g. this is how some of our decent-sized trace looks like in the trace list:
    image and
    image

    • Some integrations with http libraries prefix their outgoing request spans with Sent. and in that light, we add the counterpart Recv.<service_name> to indicate when a service starts with a request, so those indicate the boundaries between services. Without it, using the path alone, it's very hard to spot which service is doing something in these large traces.
    • Not all spans contains a correct attribute to easily identify the service / deployment which is generating the trace. Those are 'autogenerated' and from the end-user perspective it's not user-friendly to just insert themselves. The name of a span can be easily changed to what a user wants without conflicting with other attributes / labels.
  • Having a name (prefix) allows for searching traces from a certain service. I can't speak for other platforms, but StackDriver does not allow to filter on labels or attributes, only by name.

  • The opencensus Go package also does provide a hook for easily customizing the span name (https://godoc.org/go.opencensus.io/plugin/ochttp#Handler, the FormatSpanName function) which in our case looks like this:

	tracingWrapper := func(handler http.Handler) http.Handler {
		incomingSpanNamer := func(req *http.Request) string {
			return fmt.Sprintf("GoAPI.%s: %s", deployment, req.URL.Path)
		}

		return &ochttp.Handler{
			Propagation:    &propagation.HTTPFormat{},
			Handler:        handler,
			FormatSpanName: incomingSpanNamer,
		}
	}

I fully agree to have the default behavior not changed following the spec, but having a hook to allow customization like Go offers, when desired for somebody's use-case seems good.

@reyang
Copy link
Copy Markdown
Contributor

reyang commented Aug 8, 2019

Thanks for the background. Now I understand the scenario.
I'm not convinced that we want to do this in OpenCensus unless we also plan to support this in OpenTelemetry. My worry is that we end up creating more public APIs for specific scenario instead providing generic primitives.

I think what you're looking for is something that can be done at exporter level (e.g. If Stackdriver wants to format the span names in a different way, the exporter opencensus-ext-stackdriver can do it). Alternatively, we can explore if this can be done using span processor - having span processor take the span data, transform to a new name, forward to exporter.

@reyang
Copy link
Copy Markdown
Contributor

reyang commented Aug 8, 2019

@c24t @songy23 You know Stackdriver better, please help to share some insights. Thanks.

@mhindery
Copy link
Copy Markdown
Contributor Author

mhindery commented Aug 8, 2019

If you start a span yourself with start_span(), you can choose any name for it as well, according to your desired policy. I can't speak for other languages except for python & go, so I don't know whether they all have public methods for it, but in OpenCensus-Go I can choose whatever name I want both for an incoming server request and application-generated spans. Why not allow it here? Right now this is the only place in the chain of services in a request where the name cannot easily be chosen.

Having it 'hardcoded' to a view is very restrictive, especially since with the dynamic nature of python & django views; you can have several structures and hierarchies where the initial View name isn't necessarily what you want as a representation. A concrete example from one of the codebases I'm working with is

# urls.py file
    url(r'^api/v3/endpoint_cats/?', ApiView.as_view(input_generator=CatInputGenerator, <other_args_here>), name="apiv3_cats"),
    url(r'^api/v3/endpoint_dogs/?', ApiView.as_view(input_generator=DogInputGenerator, <other_args_here>), name="apiv3_dogs"),
    url(r'^api/v3/endpoint_birds/?', ApiView.as_view(input_generator=BirdsInputGenerator, <other_args_here>), name="apiv3_birds"),
    # a lot of other url entries ...

In this case ApiView is a view containing common logic for all api requests (think rate limiting, security, caching ...), but with a component as an argument which defines the use of the endpoint. All the endpoints here would have the same span name being package_name.ApiView while they should be distinct. In such a case, you should be able to use a different naming convention than the default one, as that just doesn't fit all setups. Regardless of 'you can do this example from above differently like this or that'; most of the time there are certain reasons for certain structures and setups, and this middleware is meant for all django projects so it should be able to handle these possibilities.

Copy link
Copy Markdown
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

This PR looks to me more of an enhancement to a specific framework instrumentation (Django in this case). In general I agree that hard-coded span names in the instrumentation could be restrictive. We can allow users to customize names, while default settings should still follow the HTTP specs. In terms of Stackdriver Trace I agree that adding a prefix would help with navigation. WDYT @c24t ?

@mhindery
Copy link
Copy Markdown
Contributor Author

Closing this in favor of development on the opentelemetry project: open-telemetry/opentelemetry-python#571

@mhindery mhindery closed this Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants