Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add logging, with support for filtering #280

Merged
merged 1 commit into from
Mar 5, 2017
Merged

Add logging, with support for filtering #280

merged 1 commit into from
Mar 5, 2017

Conversation

bruce
Copy link
Contributor

@bruce bruce commented Mar 5, 2017

Adds Absinthe.Logger

@bruce bruce merged commit 4f2223c into next Mar 5, 2017
@bruce bruce deleted the logger branch March 5, 2017 06:33
jparise
jparise reviewed Mar 6, 2017
View reviewed changes
lib/absinthe/logger.ex
|> Keyword.get(:filter_variables, @default_filtering)
end

@spec filter_variables(map) :: map
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be filter_variables(map, [String.t]) :: map?

lib/absinthe/logger.ex

## Variable filtering

Absinthe can filter out sensitive information like tokens and passwords
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps note that the definition of "filter out" is "replace with the string [FILTERED]"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure (this is copied almost verbatim from Phoenix, btw, to include that wording.)

lib/absinthe/logger.ex
end
end

def do_pipeline_section(pipeline) do
Copy link
Contributor

Choose a reason for hiding this comment

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

defp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, probably should be.

test/lib/absinthe/logger_test.exs
"alsoUnsafe" => @value
} = Absinthe.Logger.filter_variables(@variables)
# Change the config
Application.put_env(:absinthe, Absinthe.Logger, [filter_variables: ~w(token alsoUnsafe)])
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like I should know this, but does this line alter global state such that other test scopes would be affected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will, which is annoying. I need to find a better pattern for testing against application configuration.

@bruce
Copy link
Contributor Author

bruce commented Mar 7, 2017

Feedback addressed in #282. Thanks!

benwilson512 added a commit that referenced this pull request Apr 13, 2017
@benwilson512
v1.3: Middleware, Logger, Included Custom Types, etc (#284)
ffbec4d 
* Add logging, with support for filtering (#280)

* Support log: false configuration

* Address some feedback, clean up defaults (#282)

* Middleware (#254)

* minor optimization around arguments

* basic functionality

* almost green tests

* green tests

* handle middleware callbacks

* move plugins over to the middleware namespace

* forgot files as usual

* significant progress

* remove async and batch indirection

* rename Absinthe.Resolution.Middleware => Absinthe.Middleware

* handle before and after resolution

* some middleware testing

* improved resolution state names

* plug is now middleware

* finalize name for ensuring middleware

* doc updates

* it helps if you commit all the files

* finalizing names

* handle commentary

* better doc header

* Schema ensure query type (#226)

* Add query type rule

* Start to fix tests with querytype

* Add test

* fix more tests with query type

* Remove double imports

* Remove unused Type

* Add Explanation to FieldImportExist

* Fix the last two tests

Rewinding my own mistakes

* Fixed valid_schema

* ensure anon function form works

* context modification test

* more docs

* updated travis

* last doc tweak

* changelog update

* version bump

* correct version

* typo fix

* Date and Time Scalars (#256)

* added utc_datetime as a built-in scalar

* added naive datetime as a built-in scalar

* added date as a built-in scalar

* added time as a built-in scalar

* removed datetime types from built_ins

* moved datetime types to new extensions module

* added stricter pattern matching to datetime parser

* elixir 1.4 for travis

* fixed parsing and renamed module

* bumped elixir version

* bumped elixir version for travis

* testing against UTC offsets

* testing parsing 0-utc offset

* parsers are passed a blueprint input struct instead of a string

* added high-level tests for custom types

* build fix

* Use Markdown list syntax for nicer rendering (#285)

* changelog entry

* doc fix

* Support for `to_string/1` compatible error tuples in resolution  (#286)

* Use to_string for error messages

Sometimes you get other types in {:error, value}, like atom and integers,
and instead of people handling the converation themselves it's better just
returning the string equivalent of the atom inline with what json libraries
like Poison do or any other type that can be a string.

An example would be if you are doing `with` in you body you get the error
message from the library you are using. In guardian case you could get:
`{:error, :token_expired}`

* Changes for bruce

* Add changelog entry

* Moving TODOs to PR

* better changelog for beta

* fix bug where root query and mutation types had nil identifiers

* version bump

* provide test helping primer function

* docs

* Tuning (#292)

* do expansion prior so that we don't have to do it in resolution

* faster field applies check

* get rid of infinite loops and improved type condition handling

* add renamed file, and code reorganization

* so close to green

* tests are green at last

* remove bug line

* Fix #244

* Interface subtype (#293)

* Add failing test for interface subtype (#288)

This shows that an implementor of an interface with a more specific
field than expected will not compile.

* test fixes

* fixes to enable absinthe relay

* test prime bug fix

* more assertive

* resurrect plugin module as the before/after callbacks from middleware

* middleware.t => middleware.spec

* move scalar and enum serialization to the result phase (#296)

* remove commented out debug statements

* Add Absinthe.Pipeline.replace/3 (#300)

* Add Pipeline.replace/3

* Better type spec for Type.Directive

* Make Pipeline.replace/3 more flexible

* Clean up some warnings

* Prepare 1.3.0-beta.2

* Fix doc typo

* Another doc fix

* laxer test time for travis

* Batch resolution phase (#302)

* basic work on batching resolution phase

* bug fix

* remove warnings

* Changed HandleAuth to HandleError (#297)

* minor tweaks to support absinthe plug batching

* Extensions (#306)

* make the whole pipeline work with blueprints

* extensions test

* bug fixes

* version bump

* if a validation phase cannot jump, it returns an error instead of an ok tuple from the phase

* remove strange indentation

* fix performance issue, need to fix some tests though (#307)

* fix performance issue, need to fix some tests though

* green tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jparise jparise jparise left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bruce @jparise