Use OTel recommended names for attributes (#79)

* Use OTel recommended names for attributes

OpenTelemetry documents some conventional names to use for some pieces
of GraphQL server data. This change renames one attribute and adds two
more in line with OTel's recommendation, and also changes the name given
to the span that is started.

- Attribute "graphql.operation.name" was added
- Attribute "graphql.operation.type" was added

- BREAKING: "graphql.request.query" was renamed to "graphql.document".
- Span name was changed to `graphql.operation.type` +
  `graphql.operation.name` if those fields are present, else "GraphQL
  Operation"

See:
<https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/instrumentation/graphql/>

* Get operation name and type from blueprint

* Update changelog

* Document the trace_request_name and trace_request_type options

* Stringify the otel semantic convention attributes

Turns out tests break when we use atoms. Ideally we should look into
replacing all attributes with atoms instead, but for the sake of
simplicity this commit opts for using strings instead.

* Change behavior of the span_name config option

* Deprecate setting span_name

---------

Co-authored-by: mae.kasza <26093674+MaeIsBad@users.noreply.github.com>

CHANGELOG.md

@@ -9,15 +9,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- new `trace_request_selections` option to enable tracing root level GraphQL selections, which will be stored under `graphql.request.selections`
+- new `trace_request_selections` option to enable tracing root level GraphQL selections, which will be stored under `graphql.request.selections`.
+- attribute `graphql.operation.name` was added.
+- attribute `graphql.operation.type` was added.
+- span_name can now be set to `:dynamic`, causing it to be set dynamically based on the operation type and name, as recommended by [opentelemetry](https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/instrumentation/graphql/).
 
 ### Changed
 
-* `OpentelemetryAbsinthe.setup` can now optionally recieve the configuration. Previously `OpentelemetryAbsinthe.Instrumentation.setup` had to be used.
-* opentelemetry_absinthe will no longer log sensitive information by default.
+- BREAKING: `graphql.request.query` was renamed to `graphql.document`.
+- BREAKING: the default value of span_name is now `:dynamic`
+- BREAKING: opentelemetry_absinthe will no longer log sensitive information by default.
   By default the graphql.request.variables, graphql.response.errors and graphql.response.result attributes will no longer be emited.
   The previous behavior can be restored by setting the opentelemetry_absinthe configuration options.
 
+- `OpentelemetryAbsinthe.setup` can now optionally recieve the configuration. Previously `OpentelemetryAbsinthe.Instrumentation.setup` had to be used.
+
+### Deprecated
+- setting the span name to a static string.
+
 ## [1.1.0] - 2022-09-21
 
 ### Changed

lib/instrumentation.ex

@@ -11,6 +11,8 @@ defmodule OpentelemetryAbsinthe.Instrumentation do
   alias Absinthe.Blueprint
 
   require OpenTelemetry.Tracer, as: Tracer
+  require OpenTelemetry.SemanticConventions.Trace, as: Conventions
+  require Logger
   require Record
 
   @span_ctx_fields Record.extract(:span_ctx,
@@ -19,9 +21,16 @@ defmodule OpentelemetryAbsinthe.Instrumentation do
 
   Record.defrecord(:span_ctx, @span_ctx_fields)
 
+  @default_operation_span "GraphQL Operation"
+  @graphql_document_attr Atom.to_string(Conventions.graphql_document())
+  @graphql_operation_name_attr Atom.to_string(Conventions.graphql_operation_name())
+  @graphql_operation_type_attr Atom.to_string(Conventions.graphql_operation_type())
+
   @default_config [
-    span_name: "absinthe graphql resolution",
+    span_name: :dynamic,
     trace_request_query: true,
+    trace_request_name: true,
+    trace_request_type: true,
     trace_request_variables: false,
     trace_request_selections: true,
     trace_response_result: false,
@@ -35,6 +44,10 @@ defmodule OpentelemetryAbsinthe.Instrumentation do
       |> Keyword.merge(instrumentation_opts)
       |> Enum.into(%{})
 
+    if is_binary(config.span_name) do
+      Logger.warn("The opentelemetry_absinthe span_name option is deprecated and will be removed in the future")
+    end
+
     :telemetry.attach(
       {__MODULE__, :operation_start},
       [:absinthe, :execute, :operation, :start],
@@ -65,20 +78,35 @@ defmodule OpentelemetryAbsinthe.Instrumentation do
         config.trace_request_variables,
         {"graphql.request.variables", Jason.encode!(variables)}
       )
-      |> put_if(config.trace_request_query, {"graphql.request.query", document})
+      |> put_if(config.trace_request_query, {@graphql_document_attr, document})
 
     save_parent_ctx()
 
-    new_ctx = Tracer.start_span(config.span_name, %{attributes: attributes})
+    span_name = span_name(nil, nil, config.span_name)
+    new_ctx = Tracer.start_span(span_name, %{attributes: attributes})
 
     Tracer.set_current_span(new_ctx)
   end
 
   def handle_operation_stop(_event_name, _measurements, data, config) do
+    operation_type = get_operation_type(data)
+    operation_name = get_operation_name(data)
+
+    span_name = span_name(operation_type, operation_name, config.span_name)
+    Tracer.update_name(span_name)
+
     errors = data.blueprint.result[:errors]
 
     result_attributes =
       []
+      |> put_if(
+        config.trace_request_type,
+        {@graphql_operation_type_attr, operation_type}
+      )
+      |> put_if(
+        config.trace_request_name,
+        {@graphql_operation_name_attr, operation_name}
+      )
       |> put_if(
         config.trace_response_result,
         {"graphql.response.result", Jason.encode!(data.blueprint.result)}
@@ -114,6 +142,19 @@ defmodule OpentelemetryAbsinthe.Instrumentation do
     @default_config
   end
 
+  defp get_operation_type(%{blueprint: %Blueprint{} = blueprint}) do
+    blueprint |> Absinthe.Blueprint.current_operation() |> Kernel.||(%{}) |> Map.get(:type)
+  end
+
+  defp get_operation_name(%{blueprint: %Blueprint{} = blueprint}) do
+    blueprint |> Absinthe.Blueprint.current_operation() |> Kernel.||(%{}) |> Map.get(:name)
+  end
+
+  defp span_name(_, _, name) when is_binary(name), do: name
+  defp span_name(nil, _, _), do: @default_operation_span
+  defp span_name(op_type, nil, _), do: Atom.to_string(op_type)
+  defp span_name(op_type, op_name, _), do: "#{Atom.to_string(op_type)} #{op_name}"
+
   # Surprisingly, that doesn't seem to by anything in the stdlib to conditionally
   # put stuff in a list / keyword list.
   # This snippet is approved by José himself:

lib/opentelemetry_absinthe.ex

@@ -31,15 +31,22 @@ defmodule OpentelemetryAbsinthe do
 
   ## Configuration options
     
-    * `span_name`(default: #{Keyword.fetch!(@config, :span_name)}): the name of the span attributes will be attached to
+    * `span_name`(default: #{Keyword.fetch!(@config, :span_name)}):
+
+      Either 
+        - `:dynamic`: sets the span name dynamically, based on the operation name and type, as recommended by [opentelemetry](https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/instrumentation/graphql/). This will become the only supported option in the future.
+        - `String.t()`: *deprecated* the name of the span
+
     * `trace_request_query`(default: #{Keyword.fetch!(@config, :trace_request_query)}): attaches the graphql query as an attribute
 
       **Important Note**: This is usually safe, since graphql queries are expected to be static. All dynamic data should be passed via graphql variables. 
       However some libraries(for example [dillonkearns/elm-graphql](https://github.com/dillonkearns/elm-graphql/issues/27) store the variables inline as a part of the query.
       If you expect clients to send dynamic data as a part of the graphql query you should disable this.
 
+    * `trace_request_name`(default: #{Keyword.fetch!(@config, :trace_request_name)}): attaches the graphql operation name when using batched queries as an attribute
+    * `trace_request_type`(default: #{Keyword.fetch!(@config, :trace_request_type)}): attaches the graphql query type(query, mutation or subscription) as an attribute
     * `trace_request_variables`(default: #{Keyword.fetch!(@config, :trace_request_variables)}): attaches the graphql variables as an attribute
-    * `trace_request_selections`(default: #{Keyword.fetch!(@config, :trace_request_selections)}): attaches the root fields queried as an attribute.
+    * `trace_request_selections`(default: #{Keyword.fetch!(@config, :trace_request_selections)}): attaches the root fields queried as an attribute
     
       For example given a query like:
       ```

mix.exs

@@ -50,6 +50,7 @@ defmodule OpentelemetryAbsinthe.MixProject do
     [
       {:opentelemetry, "~> 1.1", only: :test},
       {:opentelemetry_exporter, "~> 1.1", only: :test},
+      {:opentelemetry_semantic_conventions, "~> 0.2"},
       {:credo, "~> 1.4", only: [:dev, :test]},
       {:dialyxir, "~> 1.0", only: [:dev, :test], runtime: false},
       {:ex_doc, ">= 0.0.0", only: :dev, runtime: false}

mix.lock

@@ -21,6 +21,7 @@
   "opentelemetry": {:hex, :opentelemetry, "1.1.1", "02de53d7dcafc087793ddf98cac946aaaa13c99cb6a7e568d9bb5ce4552b340e", [:rebar3], [{:opentelemetry_api, "~> 1.1", [hex: :opentelemetry_api, repo: "hexpm", optional: false]}], "hexpm", "43a807d536bca55542731ddb5ecf68c0b3b433ff98713a6496058075bac70031"},
   "opentelemetry_api": {:hex, :opentelemetry_api, "1.1.0", "156366bfbf249f54daf2626e087e29ad91201eab670993fd9ae1bd278d03a096", [:mix, :rebar3], [], "hexpm", "e0d0b49e21e5785da675c97104c385283cae84fcc0d8522932a5dcf55489ead1"},
   "opentelemetry_exporter": {:hex, :opentelemetry_exporter, "1.2.0", "c78a4fecba55a54ed2d4559f55addb88b37fc31820d7d0b2964aaebccd8b0614", [:rebar3], [{:grpcbox, ">= 0.0.0", [hex: :grpcbox, repo: "hexpm", optional: false]}, {:opentelemetry, "~> 1.1", [hex: :opentelemetry, repo: "hexpm", optional: false]}, {:opentelemetry_api, "~> 1.1", [hex: :opentelemetry_api, repo: "hexpm", optional: false]}, {:tls_certificate_check, "~> 1.11", [hex: :tls_certificate_check, repo: "hexpm", optional: false]}], "hexpm", "358d13805ef9f0521160ccca4ab533d3c8b1ca7561f72a90d3dd7ef30877c159"},
+  "opentelemetry_semantic_conventions": {:hex, :opentelemetry_semantic_conventions, "0.2.0", "b67fe459c2938fcab341cb0951c44860c62347c005ace1b50f8402576f241435", [:mix, :rebar3], [], "hexpm", "d61fa1f5639ee8668d74b527e6806e0503efc55a42db7b5f39939d84c07d6895"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"},
   "telemetry": {:hex, :telemetry, "0.4.3", "a06428a514bdbc63293cd9a6263aad00ddeb66f608163bdec7c8995784080818", [:rebar3], [], "hexpm", "eb72b8365ffda5bed68a620d1da88525e326cb82a75ee61354fc24b844768041"},
   "tls_certificate_check": {:hex, :tls_certificate_check, "1.15.0", "1c0377617a1111000bca3f4cd530b62690c9bd2dc9b868b4459203cd4d7f16ab", [:rebar3], [{:ssl_verify_fun, "1.1.6", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm", "87fd2e865078fdf8913a8c27bd8fe2be986383e31011f21d7f92cc5f7bc90731"},

test/configuration_test.exs

@@ -13,7 +13,9 @@ defmodule OpentelemetryAbsintheTest.Configuration do
       attributes = Query.query_for_attrs(Queries.query(), variables: %{"isbn" => "A1"})
 
       assert [
-               "graphql.request.query",
+               "graphql.document",
+               "graphql.operation.name",
+               "graphql.operation.type",
                "graphql.request.selections"
              ] = attributes |> Map.keys() |> Enum.sort()
     end
@@ -28,6 +30,8 @@ defmodule OpentelemetryAbsintheTest.Configuration do
       attributes = Query.query_for_attrs(Queries.query(), variables: %{"isbn" => "A1"})
 
       assert [
+               "graphql.operation.name",
+               "graphql.operation.type",
                "graphql.request.selections",
                "graphql.response.result"
              ] = attributes |> Map.keys() |> Enum.sort()
@@ -43,7 +47,9 @@ defmodule OpentelemetryAbsintheTest.Configuration do
       attributes = Query.query_for_attrs(Queries.query(), variables: %{"isbn" => "A1"})
 
       assert [
-               "graphql.request.query"
+               "graphql.document",
+               "graphql.operation.name",
+               "graphql.operation.type"
              ] = attributes |> Map.keys() |> Enum.sort()
     end
   end

test/extraction_test.exs

@@ -9,7 +9,7 @@ defmodule OpentelemetryAbsintheTest.Extraction do
       OpentelemetryAbsinthe.Instrumentation.setup(trace_request_query: true)
       query = Queries.query()
 
-      assert ^query = query |> Query.query_for_attrs() |> Map.fetch!("graphql.request.query")
+      assert ^query = query |> Query.query_for_attrs() |> Map.fetch!("graphql.document")
     end
 
     test "request variables" do

test/instrumentation_test.exs

@@ -18,7 +18,9 @@ defmodule OpentelemetryAbsintheTest.Instrumentation do
       attrs = Query.query_for_attrs(Queries.empty_query())
 
       assert [
-               "graphql.request.query",
+               "graphql.document",
+               "graphql.operation.name",
+               "graphql.operation.type",
                "graphql.request.selections",
                "graphql.request.variables",
                "graphql.response.errors",
@@ -32,7 +34,9 @@ defmodule OpentelemetryAbsintheTest.Instrumentation do
     attrs = Query.query_for_attrs(Queries.batch_queries(), variables: %{"isbn" => "A1"}, operation_name: "OperationOne")
 
     assert [
-             "graphql.request.query",
+             "graphql.document",
+             "graphql.operation.name",
+             "graphql.operation.type",
              "graphql.request.selections",
              "graphql.request.variables",
              "graphql.response.errors",

test/span_name_test.exs

@@ -0,0 +1,30 @@
+defmodule OpentelemetryAbsintheTest.SpanName do
+  use OpentelemetryAbsintheTest.Case
+
+  alias OpentelemetryAbsintheTest.Support.GraphQL.Queries
+  alias OpentelemetryAbsintheTest.Support.Query
+
+  describe "span name" do
+    test "defaults to GraphQL Operation" do
+      OpentelemetryAbsinthe.Instrumentation.setup()
+      assert "GraphQL Operation" = Query.query_for_span_name(Queries.invalid_query())
+    end
+
+    test "is the operation type for an unnamed operation" do
+      OpentelemetryAbsinthe.Instrumentation.setup()
+      assert "query" = Query.query_for_span_name(Queries.query())
+      assert "mutation" = Query.query_for_span_name(Queries.mutation(), variables: %{"isbn" => "A1"})
+    end
+
+    test "is the operation type and name for named operations" do
+      OpentelemetryAbsinthe.Instrumentation.setup()
+      assert "query OperationTwo" = Query.query_for_span_name(Queries.batch_queries(), operation_name: "OperationTwo")
+    end
+
+    test "can be overriden with the configuration" do
+      span_name = "graphql? I hardly know er"
+      OpentelemetryAbsinthe.Instrumentation.setup(span_name: span_name)
+      assert ^span_name = Query.query_for_span_name(Queries.batch_queries(), operation_name: "OperationTwo")
+    end
+  end
+end

test/support/graphql_queries.ex

@@ -55,4 +55,8 @@ defmodule OpentelemetryAbsintheTest.Support.GraphQL.Queries do
     }
     """
   end
+
+  def invalid_query do
+    "invalid_query"
+  end
 end

test/support/query.ex

@@ -18,4 +18,14 @@ defmodule OpentelemetryAbsintheTest.Support.Query do
     assert_receive {:span, span(attributes: {_, _, _, _, attributes})}, 5000
     attributes
   end
+
+  def query_for_span_name(query, opts \\ []) do
+    :otel_simple_processor.set_exporter(:otel_exporter_pid, self())
+
+    {:ok, data} = Absinthe.run(query, Schema, opts)
+    Logger.debug("Absinthe query returned: #{inspect(data)}")
+
+    assert_receive {:span, span(name: name)}, 5000
+    name
+  end
 end