From 6e6ffaff6ef61dec7f4a1d938f0953cecc5cc6e6 Mon Sep 17 00:00:00 2001 From: MaeIsBad <26093674+MaeIsBad@users.noreply.github.com> Date: Mon, 6 Mar 2023 17:08:30 +0100 Subject: [PATCH] [PLATFORM-938]: Don't log sensitive data by default (#80) --- CHANGELOG.md | 12 ++++--- README.md | 2 ++ lib/instrumentation.ex | 12 ++++--- lib/opentelemetry_absinthe.ex | 62 +++++++++++++++++++++++++++++++++-- mix.exs | 8 +++++ test/configuration_test.exs | 19 ++++------- test/extraction_test.exs | 2 +- test/instrumentation_test.exs | 12 +++++-- 8 files changed, 103 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8601741..f18660a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [1.2.0] - 2023-02-24 - ### Added - new `trace_request_selections` option to enable tracing root level GraphQL selections, which will be stored under `graphql.request.selections` +### 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. + 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. + ## [1.1.0] - 2022-09-21 ### Changed @@ -22,6 +27,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 If you're upgrading to opentelemetry_absinthe 1.1.0, it is therefore recommended to also upgrade to OpenTelemetry API 1.1.0 in order to keep the opentelemetry log metadata. -[Unreleased]: https://github.com/primait/opentelemetry_absinthe/compare/1.2.0...HEAD -[1.2.0]: https://github.com/primait/opentelemetry_absinthe/compare/1.1.0...1.2.0 +[Unreleased]: https://github.com/primait/opentelemetry_absinthe/compare/1.1.0...HEAD [1.1.0]: https://github.com/primait/opentelemetry_absinthe/releases/tag/1.1.0 diff --git a/README.md b/README.md index 54ab466..54471d3 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,5 @@ # OpentelemetryAbsinthe OpentelemetryAbsinthe is a [OpenTelemetry](https://opentelemetry.io) instrumentation library for [Absinthe](https://github.com/absinthe-graphql/absinthe). + +Full documentation is avaliable on [hexdocs](https://hexdocs.pm/opentelemetry_absinthe) diff --git a/lib/instrumentation.ex b/lib/instrumentation.ex index d5abe28..ae94900 100644 --- a/lib/instrumentation.ex +++ b/lib/instrumentation.ex @@ -22,10 +22,10 @@ defmodule OpentelemetryAbsinthe.Instrumentation do @default_config [ span_name: "absinthe graphql resolution", trace_request_query: true, - trace_request_variables: true, - trace_response_result: true, - trace_response_errors: true, - trace_request_selections: true + trace_request_variables: false, + trace_request_selections: true, + trace_response_result: false, + trace_response_errors: false ] def setup(instrumentation_opts \\ []) do @@ -110,6 +110,10 @@ defmodule OpentelemetryAbsinthe.Instrumentation do |> Enum.uniq() end + def default_config do + @default_config + end + # 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: diff --git a/lib/opentelemetry_absinthe.ex b/lib/opentelemetry_absinthe.ex index d5b5b7e..0a40244 100644 --- a/lib/opentelemetry_absinthe.ex +++ b/lib/opentelemetry_absinthe.ex @@ -1,7 +1,65 @@ defmodule OpentelemetryAbsinthe do + alias OpentelemetryAbsinthe.Instrumentation + + # Allow alias before moduledoc + # credo:disable-for-next-line @moduledoc """ - OpentelemetryAbsinthe is an opentelemetry instrumentation library for Absinthe + # OpentelemetryAbsinthe + An opentelemetry instrumentation library for Absinthe + + ## Usage + + To start collecting traces just put `OpentelemetryAbsinthe.setup()` in your application start function. + + ## Configuration + + OpentelemetryAbsinthe can be configured with the application environment + ``` + config :opentelemetry_absinthe, + trace_request_query: false, + trace_response_error: true + ``` + configuration can also be passed directly to the setup function + ``` + OpentelemetryAbsinthe.setup( + trace_request_query: false, + trace_response_error: true + ) + ``` + + ## Configuration options + + * `span_name`(default: #{Instrumentation.default_config()[:span_name]}): the name of the span attributes will be attached to + * `trace_request_query`(default: #{Instrumentation.default_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_variables`(default: #{Instrumentation.default_config()[:trace_request_variables]}): attaches the graphql variables as an attribute + * `trace_request_selections`(default: #{Instrumentation.default_config()[:trace_request_selections]}): attaches the root fields queried as an attribute. + + For example given a query like: + ``` + query($isbn: String!) { + book(isbn: $isbn) { + title + author { + name + age + } + } + reader { + name + } + } + ``` + will result in a graphql.request.selections attribute with the value `["book", "reader"]` being attached. + Note that aliased fields will use their unaliased name. + + * `trace_response_result`(default: #{Instrumentation.default_config()[:trace_request_result]}): attaches the result returned by the server as an attribute + * `trace_response_errors`(default: #{Instrumentation.default_config()[:trace_response_errors]}): attaches the errors returned by the server as an attribute """ - defdelegate setup, to: OpentelemetryAbsinthe.Instrumentation + defdelegate setup(instrumentation_opts \\ []), to: Instrumentation end diff --git a/mix.exs b/mix.exs index e73a097..097558b 100644 --- a/mix.exs +++ b/mix.exs @@ -13,6 +13,7 @@ defmodule OpentelemetryAbsinthe.MixProject do # Since absinthe is an optional dependency we need to tell dialyxir to include it plt_add_apps: [:absinthe] ], + docs: docs(), package: package(), aliases: aliases(), description: description() @@ -29,6 +30,13 @@ defmodule OpentelemetryAbsinthe.MixProject do ] end + defp docs do + [ + main: "OpentelemetryAbsinthe", + formatters: ["html"] + ] + end + defp deps do [ {:absinthe, "~> 1.7.0", optional: true}, diff --git a/test/configuration_test.exs b/test/configuration_test.exs index 2cfc9c3..d1cf194 100644 --- a/test/configuration_test.exs +++ b/test/configuration_test.exs @@ -7,24 +7,21 @@ defmodule OpentelemetryAbsintheTest.Configuration do doctest OpentelemetryAbsinthe.Instrumentation describe "trace configuration" do - test "records all graphql stuff in attributes by default" do + test "doesn't record sensitive data in attributes by default" do OpentelemetryAbsinthe.Instrumentation.setup() attributes = Query.query_for_attrs(Queries.query(), variables: %{"isbn" => "A1"}) assert [ "graphql.request.query", - "graphql.request.selections", - "graphql.request.variables", - "graphql.response.errors", - "graphql.response.result" + "graphql.request.selections" ] = attributes |> Map.keys() |> Enum.sort() end test "gives options provided via application env have precedence over defaults" do Application.put_env(:opentelemetry_absinthe, :trace_options, trace_request_query: false, - trace_response_result: false + trace_response_result: true ) OpentelemetryAbsinthe.Instrumentation.setup() @@ -32,25 +29,21 @@ defmodule OpentelemetryAbsintheTest.Configuration do assert [ "graphql.request.selections", - "graphql.request.variables", - "graphql.response.errors" + "graphql.response.result" ] = attributes |> Map.keys() |> Enum.sort() end test "gives options provided to setup() precedence over defaults and application env" do Application.put_env(:opentelemetry_absinthe, :trace_options, trace_request_query: false, - trace_response_result: false + trace_request_selections: false ) OpentelemetryAbsinthe.Instrumentation.setup(trace_request_query: true) attributes = Query.query_for_attrs(Queries.query(), variables: %{"isbn" => "A1"}) assert [ - "graphql.request.query", - "graphql.request.selections", - "graphql.request.variables", - "graphql.response.errors" + "graphql.request.query" ] = attributes |> Map.keys() |> Enum.sort() end end diff --git a/test/extraction_test.exs b/test/extraction_test.exs index eab75bf..b4d5924 100644 --- a/test/extraction_test.exs +++ b/test/extraction_test.exs @@ -70,7 +70,7 @@ defmodule OpentelemetryAbsintheTest.Extraction do end test "response result" do - OpentelemetryAbsinthe.Instrumentation.setup(trace_response_results: true) + OpentelemetryAbsinthe.Instrumentation.setup(trace_response_result: true) result = Queries.query() diff --git a/test/instrumentation_test.exs b/test/instrumentation_test.exs index d1c0d8a..3f6ac0c 100644 --- a/test/instrumentation_test.exs +++ b/test/instrumentation_test.exs @@ -4,9 +4,17 @@ defmodule OpentelemetryAbsintheTest.Instrumentation do alias OpentelemetryAbsintheTest.Support.GraphQL.Queries alias OpentelemetryAbsintheTest.Support.Query + @capture_all [ + trace_request_query: true, + trace_request_variables: true, + trace_response_result: true, + trace_response_errors: true, + trace_request_selections: true + ] + describe "query" do test "doesn't crash when empty" do - OpentelemetryAbsinthe.Instrumentation.setup() + OpentelemetryAbsinthe.Instrumentation.setup(@capture_all) attrs = Query.query_for_attrs(Queries.empty_query()) assert [ @@ -20,7 +28,7 @@ defmodule OpentelemetryAbsintheTest.Instrumentation do end test "handles multiple queries properly" do - OpentelemetryAbsinthe.Instrumentation.setup() + OpentelemetryAbsinthe.Instrumentation.setup(@capture_all) attrs = Query.query_for_attrs(Queries.batch_queries(), variables: %{"isbn" => "A1"}, operation_name: "OperationOne") assert [