Add stackdriver project sink support #432
Conversation
|
|
||
| # google\_logging\_project\_sink | ||
|
|
||
| Manages a project-level logging sink.. For more information see |
| Name: d.Get("name").(string), | ||
| Destination: d.Get("destination").(string), | ||
| } | ||
| if v, ok := d.GetOk("filter"); ok { |
There was a problem hiding this comment.
Do you need this if block? d.Get("filter") will return "" and should work without this extra if block
There was a problem hiding this comment.
That's easier to read. Fixed.
google/logging_utils.go
Outdated
| // LoggingSinkId represents the parts that make up the canonical id used within terraform for a logging resource. | ||
| type LoggingSinkId struct { | ||
| typ string | ||
| typName string |
There was a problem hiding this comment.
In the docs, the parent parameter documentation says:
The resource in which to create the sink:
"projects/[PROJECT_ID]"
"organizations/[ORGANIZATION_ID]"
"billingAccounts/[BILLING_ACCOUNT_ID]"
"folders/[FOLDER_ID]"Instead of type, we could use resource.
resourceType instead of typ
resourceId instead of typName
There was a problem hiding this comment.
Your names are much better! Fixed.
|
Very good. Just some minor typos and suggestions. |
| config := meta.(*Config) | ||
|
|
||
| sink, err := config.clientLogging.Projects.Sinks.Get(d.Id()).Do() | ||
| if err != nil { |
There was a problem hiding this comment.
What about 404 handling? (https://github.com/terraform-providers/terraform-provider-google/blob/master/google/utils.go#L229)
| return nil | ||
| } | ||
|
|
||
| func testAccCheckLoggingProjectSinkExists(n string, sink *logging.LogSink) resource.TestCheckFunc { |
There was a problem hiding this comment.
Since everything this function does is also in the next one, is it worth it to keep them separate?
There was a problem hiding this comment.
I kept them separate for now since testAccCheckLoggingProjectSinkExists assigns the found api object to a provided *logging.LogSink parameter and testAccCheckLoggingProjectSink checks the remote object against the locally stored attributes
However, since they both look up the api object I changed testAccCheckLoggingProjectSink to take an api object as the first parameter; now it does less duplicate work
| } | ||
|
|
||
| func testAccLoggingProjectSink_basic(name, bucketName string) string { | ||
| return fmt.Sprintf(`resource "google_logging_project_sink" "basic" { |
There was a problem hiding this comment.
can you put a line break after the `? (makes it easier to see what's going on and copy and paste too) (here and elsewhere)
|
|
||
| resource "google_storage_bucket" "log-bucket" { | ||
| name = "%s" | ||
| }`, name, getTestProjectFromEnv(), bucketName) |
There was a problem hiding this comment.
likewise, line break before the `
| }) | ||
| } | ||
|
|
||
| func TestAccLoggingProjectSink_updateUniqueWriter(t *testing.T) { |
There was a problem hiding this comment.
Isn't this updating the bucket (/ the destination), not the unique writer?
There was a problem hiding this comment.
Yeah, it is. The test is poorly named. I changed it to '_updatePreservesUniqueWriter' (as that's explicitly tested)
| resource "google_logging_project_sink" "my-sink" { | ||
| name = "my-pubsub-instance-sink" | ||
|
|
||
| // Can export to pubsub, cloud storage, or bigtable |
There was a problem hiding this comment.
https://www.terraform.io/docs/configuration/syntax.html
Single line comments start with #
There was a problem hiding this comment.
Hah apparently it also accepts // as i've been using them in my hcl unknowingly for a while!
Changed to #
selmanj
force-pushed
the
add_stackdriver_project_sink
branch
from
4a15dd2 to
41415ec
Compare
* Vendor cloud logging api * Add logging sink support * Remove typo * Set Filter simpler * Rename typ, typName to resourceType, resourceId * Handle notFoundError * Use # instead of // for hcl comments * Cleanup test code * Change testAccCheckLoggingProjectSink to take a provided api object * Fix whitespace change after merge conflict
* Vendor cloud logging api * Add logging sink support * Remove typo * Set Filter simpler * Rename typ, typName to resourceType, resourceId * Handle notFoundError * Use # instead of // for hcl comments * Cleanup test code * Change testAccCheckLoggingProjectSink to take a provided api object * Fix whitespace change after merge conflict
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
This adds support for management of Stackdriver Logging sinks. The use case is to setup logging exports to buckets, pubsub topics or bigtable datasets.
See the example in the
logging_project_sink.html.markdownfile for an example of using terraform to manage logging a VM.Adding followup support for logging folders, organizations, billingAccounts etc should be easier once this initial sink is checked in.
Documentation: https://cloud.google.com/logging/docs/api/tasks/exporting-logs