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 support for Stackdriver Logging #1122

Merged
merged 32 commits into from
Jul 21, 2016
Merged

Add support for Stackdriver Logging #1122

merged 32 commits into from
Jul 21, 2016

Conversation

mziccard
Copy link
Contributor

This PR rebases commits form logging-alpha branch on top of master branch. All commits but the last 3 have already been reviewed.

pongad and others added 28 commits July 20, 2016 13:16
@pongad @mziccard
Logging API. Add generated files.
ad97ea5
@pongad @mziccard
Removed unnecessary files.
b360f6d 
These files were missed in the last commit.
@pongad @mziccard
Add Logging APIs generated by VKit.
015c398 
Some tests are written but definitely not completed yet.
It shows, however, that the API is not dead on arrival.
@garrettjonesgoogle @mziccard
Removing GAX from gcloud-java
20c1f38
@shinfan @mziccard
Update and fix gcloud-java-logging
de7fdef
@michaelbausor @mziccard
Regenerated logging service (googleapis#893)
3e9ff9d 
Regenerated java logging service and update pom to reference gax 0.0.9
@garrettjonesgoogle @mziccard
Rename com.google.gcloud to com.google.cloud (googleapis#908)
2e5d57c
@shinfan @mziccard
Update logging with latest generator and GAX. (googleapis#1046)
52d5121 
* Update logging with latest generator.

* Update comments

* Update grpc dependencies.

* Update local logging implementation.

* Remove gax related files and .classpath
@mziccard
Update logging module version and dependencies
39a8fb9
@mziccard
Implement spi layer for Logging service (googleapis#1056)
444e742
@mziccard
Add SinkInfo class and tests (googleapis#1058)
e4ae0c4
@mziccard
Add MetricInfo class and tests (googleapis#1059)
16773ff
@mziccard
Add Sink class, implement sink methods and tests (googleapis#1063)
80095e6
@mziccard
Add Metric class, implement metric methods and tests (googleapis#1091)
6bcf722
@mziccard
Add MonitoredResourceDescriptor class and tests (googleapis#1062)
b83bae7 
* Add MonitoredResourceDescriptor class and tests

* Move MonitoredResourceDescriptor to core module
@mziccard
Implement MonitoredResourceDescriptor methods and tests (googleapis#1098
6b4ace7 
)
@mziccard
Add MonitoredResource class and tests (googleapis#1097)
5555669
@mziccard
Add HttpRequest class and tests (googleapis#1099)
d8c6a09
@mziccard
Add log entry Payload class and tests (googleapis#1102)
823782f
@mziccard
Add log entry's Operation class and tests (googleapis#1104)
6aa779c
@mziccard
Add deleteLog(String) method and tests (googleapis#1105)
08080f0
@mziccard
Add LogEntry class, Severity enum and tests (googleapis#1106)
a5c98b2
@mziccard
Implement LogEntry methods and tests (googleapis#1110)
e90c772
@mziccard
Add examples and snippets for Logging (googleapis#1113)
d9320da
@mziccard
Add RemoteLoggingHelper and integration tests (googleapis#1111)
6f5e3af
@mziccard
Close service instance in Logging snippets (googleapis#1118)
e1ee489
@mziccard
Add SerializationTest for logging and minor fixes (googleapis#1116)
3eca483
@mziccard
Add JUL handlers for Logging, examples and tests (googleapis#1117)
f9a2d1a
@mziccard mziccard added the api: logging Issues related to the Cloud Logging API. label Jul 20, 2016
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Jul 20, 2016
@mziccard mziccard force-pushed the add-logging-support branch from f923245 to c8806b5 Compare July 20, 2016 14:07
@mziccard mziccard force-pushed the add-logging-support branch from c8806b5 to 1842375 Compare July 20, 2016 14:29
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 85.865% when pulling 1842375 on mziccard:add-logging-support into 05795ca on GoogleCloudPlatform:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 85.871% when pulling 1842375 on mziccard:add-logging-support into 05795ca on GoogleCloudPlatform:master.

@mziccard
Copy link
Contributor Author

@lesv can you have a look at the following commits:

7168fff: renames all instances of "Cloud Logging" to "Stackdriver Logging"
a783200: re-orders service list in examples README and add LoggingExample to appassembler config
1842375: adds logging service to main README and re-order services in TESTING.md
Also please notice that Codacy comments are on auto-generated code that we should not fix by hand in this PR.

@lesv
Copy link
Contributor

lesv commented Jul 20, 2016

Does the example app get tested anywhere?

@lesv
Copy link
Contributor

lesv commented Jul 20, 2016

Codacy seems to have a bunch of issues at on the surface seem relevant. Could you take a look and comment why we shouldn't fix them?

Also, if you could respond to the CLA bot. Curious why it's unhappy - it usually likes you.

It's probably too late for this, but it would seem like in an ideal world we'd do a jar (maven artifact) for writers, and a separate one for reading. Since most apps will be doing writing.

I thought I commented on one of the branches, but I can't find it now -- it was ignorable -- question about Cloud Logging vs. Stackdriver Logging.

LGTM once you address Codacy's comments.

@mziccard
Copy link
Contributor Author

Thanks for the pass!

Does the example app get tested anywhere?

Examples are not tested. We have this issue open that asks for example testing.

Codacy seems to have a bunch of issues at on the surface seem relevant. Could you take a look and comment why we shouldn't fix them?

I had a look at the list of issues here. I confirm that we have the following kind of issues:

  • Issues with classes in com.google.cloud.logging.spi.v2.*: these classes are autogenerated so we should/can not address the issues here
  • println or printf used in examples classes: we can ignore this error, prints are used for the sake of example
  • Examples classes contain only static methods: as above, we can ignore this
  • Other errors: these are all other errors that we decided to ignore when merging single PRs to the logging-alpha branch

Also, if you could respond to the CLA bot. Curious why it's unhappy - it usually likes you.

This PR contains commits non only for me but also from other people, this is the reason CLA bot is unhappy. Luckily all other authors are googlers and already signed the CLA so CLA is ok in reality.

It's probably too late for this, but it would seem like in an ideal world we'd do a jar (maven artifact) for writers, and a separate one for reading. Since most apps will be doing writing.

We discussed about this also on Pub/Sub but we decided to stick to one single surface for the whole service. Reasons are mainly:

  • With the amount of services we need to support doing too much fragmentation might just be confusing. A 1:1 correspondence of our modules to Cloud services is easier to understand.
  • Consistency with other gcloud-java modules
  • Consistency with other gcloud-* libraries
  • It's not straightforward to decide whether some functionality should go in the "writer" or "reader" artifact. Where should the Metric-related functionality go for instance?

I thought I commented on one of the branches, but I can't find it now -- it was ignorable -- question about Cloud Logging vs. Stackdriver Logging.

I found it. Following this issue we should always name it Stackdriver Logging for consistency with service docs.

@mziccard mziccard removed the cla: no This human has *not* signed the Contributor License Agreement. label Jul 21, 2016
@mziccard mziccard merged commit 944f366 into googleapis:master Jul 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: logging Issues related to the Cloud Logging API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mziccard @googlebot @coveralls @lesv @pongad @garrettjonesgoogle @shinfan @michaelbausor