Expand logging support for HTTP traffic #60
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "strings" | ||
|
|
||
| "github.com/cli/go-gh/pkg/jsonpretty" | ||
| ) | ||
|
|
||
| type graphqlBody struct { | ||
| Query string `json:"query"` | ||
| OperationName string `json:"operationName"` | ||
| Variables json.RawMessage `json:"variables"` | ||
| } | ||
|
|
||
| // jsonFormatter is a httpretty.Formatter that prettifies JSON payloads and GraphQL queries. | ||
| type jsonFormatter struct { | ||
| colorize bool | ||
| } | ||
|
|
||
| func (f *jsonFormatter) Format(w io.Writer, src []byte) error { | ||
| var graphqlQuery graphqlBody | ||
| // TODO: find more precise way to detect a GraphQL query from the JSON payload alone | ||
| if err := json.Unmarshal(src, &graphqlQuery); err == nil && graphqlQuery.Query != "" && len(graphqlQuery.Variables) > 0 { | ||
| colorHighlight := "\x1b[35;1m" | ||
| colorReset := "\x1b[m" | ||
| if !f.colorize { | ||
| colorHighlight = "" | ||
| colorReset = "" | ||
| } | ||
| if _, err := fmt.Fprintf(w, "%sGraphQL query:%s\n%s\n", colorHighlight, colorReset, strings.ReplaceAll(strings.TrimSpace(graphqlQuery.Query), "\t", " ")); err != nil { | ||
| return err | ||
| } | ||
| if _, err := fmt.Fprintf(w, "%sGraphQL variables:%s %s\n", colorHighlight, colorReset, string(graphqlQuery.Variables)); err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
| return jsonpretty.Format(w, bytes.NewReader(src), " ", f.colorize) | ||
| } | ||
|
|
||
| func (f *jsonFormatter) Match(t string) bool { | ||
| return jsonTypeRE.MatchString(t) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| package jsonpretty | ||
|
There was a problem hiding this comment. Do we want this package to be exported? If so, can we add a comment to the top here describing the purpose and functionality of the package? |
||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "strings" | ||
| ) | ||
|
|
||
| const ( | ||
| colorDelim = "\x1b[1;38m" // bright white | ||
| colorKey = "\x1b[1;34m" // bright blue | ||
| colorNull = "\x1b[36m" // cyan | ||
| colorString = "\x1b[32m" // green | ||
| colorBool = "\x1b[33m" // yellow | ||
| colorReset = "\x1b[m" | ||
| ) | ||
|
|
||
| // Format reads JSON from r and writes a prettified version of it to w. | ||
| func Format(w io.Writer, r io.Reader, indent string, colorize bool) error { | ||
| dec := json.NewDecoder(r) | ||
| dec.UseNumber() | ||
|
|
||
| c := func(ansi string) string { | ||
| if !colorize { | ||
| return "" | ||
| } | ||
| return ansi | ||
| } | ||
|
|
||
| var idx int | ||
| var stack []json.Delim | ||
|
|
||
| for { | ||
| t, err := dec.Token() | ||
| if err == io.EOF { | ||
| break | ||
| } | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| switch tt := t.(type) { | ||
| case json.Delim: | ||
| switch tt { | ||
| case '{', '[': | ||
| stack = append(stack, tt) | ||
| idx = 0 | ||
| if _, err := fmt.Fprint(w, c(colorDelim), tt, c(colorReset)); err != nil { | ||
| return err | ||
| } | ||
| if dec.More() { | ||
| if _, err := fmt.Fprint(w, "\n", strings.Repeat(indent, len(stack))); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| continue | ||
| case '}', ']': | ||
| stack = stack[:len(stack)-1] | ||
| idx = 0 | ||
| if _, err := fmt.Fprint(w, c(colorDelim), tt, c(colorReset)); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| default: | ||
| b, err := marshalJSON(tt) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| isKey := len(stack) > 0 && stack[len(stack)-1] == '{' && idx%2 == 0 | ||
| idx++ | ||
|
|
||
| var color string | ||
| if isKey { | ||
| color = colorKey | ||
| } else if tt == nil { | ||
| color = colorNull | ||
| } else { | ||
| switch t.(type) { | ||
| case string: | ||
| color = colorString | ||
| case bool: | ||
| color = colorBool | ||
| } | ||
| } | ||
|
|
||
| if color != "" { | ||
| if _, err := fmt.Fprint(w, c(color)); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| if _, err := w.Write(b); err != nil { | ||
| return err | ||
| } | ||
| if color != "" { | ||
| if _, err := fmt.Fprint(w, c(colorReset)); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if isKey { | ||
| if _, err := fmt.Fprint(w, c(colorDelim), ":", c(colorReset), " "); err != nil { | ||
| return err | ||
| } | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| if dec.More() { | ||
| if _, err := fmt.Fprint(w, c(colorDelim), ",", c(colorReset), "\n", strings.Repeat(indent, len(stack))); err != nil { | ||
| return err | ||
| } | ||
| } else if len(stack) > 0 { | ||
| if _, err := fmt.Fprint(w, "\n", strings.Repeat(indent, len(stack)-1)); err != nil { | ||
| return err | ||
| } | ||
| } else { | ||
| if _, err := fmt.Fprint(w, "\n"); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // marshalJSON works like json.Marshal, but with HTML-escaping disabled. | ||
| func marshalJSON(v interface{}) ([]byte, error) { | ||
| buf := bytes.Buffer{} | ||
| enc := json.NewEncoder(&buf) | ||
| enc.SetEscapeHTML(false) | ||
| if err := enc.Encode(v); err != nil { | ||
| return nil, err | ||
| } | ||
| bb := buf.Bytes() | ||
| // omit trailing newline added by json.Encoder | ||
| if len(bb) > 0 && bb[len(bb)-1] == '\n' { | ||
| return bb[:len(bb)-1], nil | ||
| } | ||
| return bb, nil | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thinking about this more, I am not sure that this option is necessary. I understand its purpose, but can't think of a good use case where we want to allow an extension author to ignore
GH_DEBUG.Additionally, I think our users would find the behavior of this option a bit confusing without further comments describing its behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for chiming in. One consumer of go-gh that wants to use this option is the CLI itself, since GitHub CLI already parses environment variables itself and wants to prevent underlying libraries to parse the same environment variables, lest some inconsistencies or conflicts arise.
Additionally, an extension author might want to disable env-controlled logging if it already offers the user granular controls for logging.
As a general rule of thumb, if a generic Go library automatically respects environment variables, it's a good idea to offer a way for the consumer of said library to skip respecting environment variables in cases where that is not desired. At the end of the day, environment variables are essentially global variables shared across modules and even processes, and to mitigate their potential harms, developers should have ways to put up boundaries to limit the propagation (or effect) of these variables.
I have added some extra documentation detailing this configuration option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That all makes sense to me, my only thought was that if an extension author wanted to ignore
GH_DEBUGthey could provide a value for theLogoption makingLogIgnoreEnvfeel a bit redundant.