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

Query passthrough changes for Cloudwatch connector #1906

Merged
merged 16 commits into from
Apr 12, 2024
Merged

Query passthrough changes for Cloudwatch connector #1906

merged 16 commits into from
Apr 12, 2024

Conversation

Trianz-Akshay
Copy link
Contributor

Description of changes:
Query passthrough changes for Cloudwatch connector
QPT syntax :

  • In case of single Log group name
    SELECT * FROM TABLE(system.query("STARTTIME" => '1710918615308', "ENDTIME" => '1710918615972', "QUERYSTRING" => '', "LOGGROUPNAMES" => '/aws/lambda/cloudwatch-test-1', "LIMIT" => '2'))

-in case of multiple log group name
SELECT * FROM TABLE(system.query("STARTTIME" => '1710918615308', "ENDTIME" => '1710918615972', "QUERYSTRING" => '', "LOGGROUPNAMES" => '"/aws/lambda/cloudwatch-test-1", "/aws/lambda/cloudwatch-test-2"', "LIMIT" => '2'))

CloudWatch Logs Insights query syntax(https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/CWL_QuerySyntax.html)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

aimethed
aimethed reviewed Apr 11, 2024
View reviewed changes
Copy link
Contributor

@aimethed aimethed left a comment

Choose a reason for hiding this comment

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

There are a lot of logging statements left in, is this intentional or leftover from testing?

@Trianz-Akshay
Copy link
Contributor Author

There are a lot of logging statements left in, is this intentional or leftover from testing?

removed leftover loggers.

chngpe
chngpe reviewed Apr 12, 2024
View reviewed changes
...atch/src/main/java/com/amazonaws/athena/connectors/cloudwatch/CloudwatchMetadataHandler.java Outdated
else {
return new GetTableResponse(request.getCatalogName(),
request.getTableName(),
CLOUDWATCH_SCHEMA);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if i like the idea of return basic schema for GetQueryPassThrough if we failed to get qpt result. Any particular reason on why we are doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not any particular reason. it is used just to provide similar behaviour as that of getTable method. I have removed that.

chngpe
chngpe reviewed Apr 12, 2024
View reviewed changes
...dwatch/src/main/java/com/amazonaws/athena/connectors/cloudwatch/CloudwatchRecordHandler.java Outdated
if (recordsRequest.getConstraints().isQueryPassThrough()) {
Map<String, String> qptArguments = recordsRequest.getConstraints().getQueryPassthroughArguments();
queryPassthrough.verify(qptArguments);
GetQueryResultsResult getQueryResultsResult = getResult(invoker, awsLogs, qptArguments, Integer.parseInt(qptArguments.get(CloudwatchQueryPassthrough.LIMIT)));
Copy link
Contributor

Choose a reason for hiding this comment

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

is "limit" a required parameter for QPT request on CW?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, can we separate QPT read record to another method? to make it easier to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

is "limit" a required parameter for QPT request on CW?

Can it be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as we are calling verify, so all args are required. so in this case it will never be null. In one more case, we are taking limit as required arg is to avoid timeout in case of cloudwatch API call.
but in Cloudwatch stsrtQuery API, the limit is optional.

separated the QPT read record to another method.

@aimethed aimethed merged commit cf9ae17 into awslabs:master Apr 12, 2024
6 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 16, 2024
Cut release 2024.15.1
2bf9460 
  - build(deps): bump slf4j-log4j.version from 2.0.7 to 2.0.13 (#1910)
  - build(deps): bump slf4j-log4j.version from 2.0.7 to 2.0.13
  - build(deps): bump software.amazon.jsii:jsii-runtime from 1.96.0 to 1.97.0 (#1914)
  - build(deps): bump software.amazon.jsii:jsii-runtime
  - build(deps): bump software.amazon.awssdk:bom from 2.25.26 to 2.25.31 (#1912)
  - build(deps): bump software.amazon.awssdk:bom from 2.25.26 to 2.25.31
  - build(deps): bump org.apache.maven.plugins:maven-gpg-plugin from 3.2.2 to 3.2.3 (#1913)
  - build(deps): bump org.apache.maven.plugins:maven-gpg-plugin
  - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client from 8.13.1 to 8.13.2 (#1916)
  - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client
  - build(deps): bump com.sap.cloud.db.jdbc:ngdbc from 2.20.11 to 2.20.17 (#1915)
  - build(deps): bump com.sap.cloud.db.jdbc:ngdbc from 2.20.11 to 2.20.17
  - build(deps): bump org.testng:testng from 7.10.0 to 7.10.1 (#1911)
  - build(deps): bump org.testng:testng from 7.10.0 to 7.10.1
  - build(deps): bump org.apache.maven.plugins:maven-jar-plugin from 3.3.0 to 3.4.0 (#1909)
  - build(deps): bump org.apache.maven.plugins:maven-jar-plugin
  - build(deps): bump aws-sdk.version from 1.12.696 to 1.12.701 (#1908)
  - build(deps): bump aws-sdk.version from 1.12.696 to 1.12.701
  - Qpt vertica changes (#1853)
  - Query passthrough changes for Cloudwatch connector (#1906)
  - Extended QPT to athena-neptune for property graph (#1886)
  - Extended QPT to athena-hbase (#1876)
  - build(deps): bump org.apache.maven.plugins:maven-source-plugin from 3.3.0 to 3.3.1 (#1897)
  - build(deps): bump org.apache.maven.plugins:maven-source-plugin
  - build(deps): bump software.amazon.awssdk:bom from 2.25.21 to 2.25.26 (#1893)
  - build(deps): bump software.amazon.awssdk:bom from 2.25.21 to 2.25.26
  - build(deps): bump com.microsoft.sqlserver:mssql-jdbc from 12.6.1.jre11 to 12.7.0.jre11-preview (#1900)
  - build(deps): bump com.microsoft.sqlserver:mssql-jdbc
  - build(deps): bump io.grpc:grpc-api from 1.62.2 to 1.63.0 (#1891)
  - build(deps): bump io.grpc:grpc-api from 1.62.2 to 1.63.0
  - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client from 8.13.0 to 8.13.1 (#1896)
  - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client
  - build(deps): bump org.jacoco:jacoco-maven-plugin from 0.8.11 to 0.8.12 (#1899)
  - build(deps): bump org.jacoco:jacoco-maven-plugin from 0.8.11 to 0.8.12
  - build(deps): bump net.snowflake:snowflake-jdbc from 3.15.0 to 3.15.1 (#1898)
  - build(deps): bump net.snowflake:snowflake-jdbc from 3.15.0 to 3.15.1
  - build(deps): bump com.github.spotbugs:spotbugs-annotations from 4.8.3 to 4.8.4 (#1901)
  - build(deps): bump com.github.spotbugs:spotbugs-annotations
  - build(deps-dev): bump nl.jqno.equalsverifier:equalsverifier from 3.16 to 3.16.1 (#1894)
  - build(deps-dev): bump nl.jqno.equalsverifier:equalsverifier
  - build(deps): bump com.google.cloud:google-cloud-resourcemanager from 1.41.0 to 1.43.0 (#1892)
  - build(deps): bump com.google.cloud:google-cloud-resourcemanager
  - build(deps): bump software.amazon.msk:aws-msk-iam-auth from 2.0.3 to 2.1.0 (#1895)
  - build(deps): bump software.amazon.msk:aws-msk-iam-auth
  - build(deps): bump aws-sdk.version from 1.12.691 to 1.12.696 (#1890)
  - build(deps): bump aws-sdk.version from 1.12.691 to 1.12.696
  - build(deps): bump org.testng:testng from 7.9.0 to 7.10.0 (#1889)
  - build(deps): bump org.testng:testng from 7.9.0 to 7.10.0
  - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client from 8.12.2 to 8.13.0 (#1882)
  - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client
  - build(deps): bump com.google.cloud:google-cloud-resourcemanager from 1.40.0 to 1.41.0 (#1883)
  - build(deps): bump com.google.cloud:google-cloud-resourcemanager
  - build(deps): bump com.microsoft.azure:msal4j from 1.14.3 to 1.15.0 (#1880)
  - build(deps): bump com.microsoft.azure:msal4j from 1.14.3 to 1.15.0
  - build(deps): bump software.amazon.awssdk:bom from 2.25.17 to 2.25.21 (#1879)
  - build(deps): bump software.amazon.awssdk:bom from 2.25.17 to 2.25.21
  - build(deps): bump aws-sdk.version from 1.12.686 to 1.12.691 (#1878)
  - build(deps): bump aws-sdk.version from 1.12.686 to 1.12.691
  - qpt change for elastic search (#1854)
  - Extended QPT to athena-sqlserver (#1823)
  - build(deps): bump software.amazon.jsii:jsii-runtime from 1.95.0 to 1.96.0 (#1869)
  - build(deps): bump software.amazon.jsii:jsii-runtime
  - build(deps): bump org.apache.maven.plugins:maven-compiler-plugin from 3.12.1 to 3.13.0 (#1867)
  - build(deps): bump org.apache.maven.plugins:maven-compiler-plugin
  - build(deps): bump software.amazon.awssdk:bom from 2.25.11 to 2.25.17 (#1874)
  - build(deps): bump software.amazon.awssdk:bom from 2.25.11 to 2.25.17
  - build(deps): bump org.apache.maven.plugins:maven-assembly-plugin from 3.7.0 to 3.7.1 (#1870)
  - build(deps): bump org.apache.maven.plugins:maven-assembly-plugin
  - build(deps-dev): bump nl.jqno.equalsverifier:equalsverifier from 3.15.8 to 3.16 (#1866)
  - build(deps-dev): bump nl.jqno.equalsverifier:equalsverifier
  - build(deps): bump org.apache.maven.plugins:maven-gpg-plugin from 3.2.0 to 3.2.2 (#1875)
  - build(deps): bump org.apache.maven.plugins:maven-gpg-plugin
  - build(deps): bump com.sap.cloud.db.jdbc:ngdbc from 2.19.16 to 2.20.11 (#1864)
  - build(deps): bump com.sap.cloud.db.jdbc:ngdbc from 2.19.16 to 2.20.11
  - build(deps): bump org.apache.hadoop:hadoop-common from 3.3.6 to 3.4.0 (#1871)
  - build(deps): bump org.apache.hadoop:hadoop-common from 3.3.6 to 3.4.0
  - build(deps): bump com.google.cloud:google-cloud-storage from 2.36.0 to 2.36.1 (#1863)
  - build(deps): bump com.google.cloud:google-cloud-storage
  - build(deps): bump aws-sdk.version from 1.12.681 to 1.12.686 (#1862)
  - build(deps): bump aws-sdk.version from 1.12.681 to 1.12.686
  - build(deps): bump dependabot/fetch-metadata from 1 to 2 (#1873)
  - build(deps): bump dependabot/fetch-metadata from 1 to 2
  - Extended QPT to athena-Synapse (#1831)
  - Extended QPT To Datalakegen2 (#1822)
  - Extended QPT to athena-db2 (#1825)
  - Extended QPT to athena-oracle (#1827)
  - Extended QPT to athena-Saphana (#1828)
  - Extended QPT to athena-teradata (#1830)
  - Extended QPT to athena-hortonworks-hive (#1824)
  - Extended QPT to athena-cloudera-hive (#1826)
  - Extended QPT to athena-snowflake (#1829)
  - Extended QPT to athena-Impala (#1832)
  - Extended QPT to athena-db2-as400 (#1833)
  - Extended QPT to athena-timestream (#1857)
  - build(deps): bump com.google.cloud:google-cloud-resourcemanager from 1.39.0 to 1.40.0 (#1861)
  - build(deps): bump com.google.cloud:google-cloud-resourcemanager
  - Extended QPT to athena-dynamodb (#1819)
  - SPILL_QUEUE_CAPACITY config property can now be set as all lowercase (#1852)
  - Extend QPT to Postgresql & Redshift (#1820)
  - build(deps): bump org.apache.commons:commons-configuration2 from 2.10.0 to 2.10.1 in /athena-neptune (#1855)
  - build(deps): bump org.apache.commons:commons-configuration2
  - Feature: Enable/Disable kerberos authentication for cloudera manager hbase instance (#1793)
  - build(deps): bump io.grpc:grpc-api from 1.61.0 to 1.62.2 (#1844)
  - build(deps): bump io.grpc:grpc-api from 1.61.0 to 1.62.2
  - build(deps): bump org.apache.maven.plugins:maven-gpg-plugin from 3.1.0 to 3.2.0 (#1843)
  - build(deps): bump org.apache.maven.plugins:maven-gpg-plugin
  - build(deps): bump com.google.cloud:google-cloud-storage from 2.35.0 to 2.36.0 (#1846)
  - build(deps): bump com.google.cloud:google-cloud-storage
  - build(deps): bump net.jqwik:jqwik from 1.8.3 to 1.8.4 (#1841)
  - build(deps): bump net.jqwik:jqwik from 1.8.3 to 1.8.4
  - build(deps): bump org.apache.commons:commons-configuration2 from 2.9.0 to 2.10.0 (#1845)
  - build(deps): bump org.apache.commons:commons-configuration2
  - build(deps): bump org.postgresql:postgresql from 42.7.2 to 42.7.3 (#1847)
  - build(deps): bump org.postgresql:postgresql from 42.7.2 to 42.7.3
  - build(deps): bump fasterxml.jackson.version from 2.16.2 to 2.17.0 (#1837)
  - build(deps): bump fasterxml.jackson.version from 2.16.2 to 2.17.0
  - build(deps): bump io.lettuce:lettuce-core from 6.3.1.RELEASE to 6.3.2.RELEASE (#1839)
  - build(deps): bump io.lettuce:lettuce-core
  - build(deps): bump software.amazon.awssdk:bom from 2.25.4 to 2.25.11 (#1842)
  - build(deps): bump software.amazon.awssdk:bom from 2.25.4 to 2.25.11
  - build(deps): bump hbase.version from 2.5.7-hadoop3 to 2.5.8-hadoop3 (#1838)
  - build(deps): bump hbase.version from 2.5.7-hadoop3 to 2.5.8-hadoop3
  - build(deps): bump aws-sdk.version from 1.12.676 to 1.12.681 (#1835)
  - build(deps): bump aws-sdk.version from 1.12.676 to 1.12.681
  - build(deps): bump com.google.guava:guava from 33.0.0-jre to 33.1.0-jre (#1836)
  - build(deps): bump com.google.guava:guava from 33.0.0-jre to 33.1.0-jre
  - build(deps): bump org.apache.zookeeper:zookeeper from 3.9.1 to 3.9.2 in /athena-hbase (#1834)
  - build(deps): bump org.apache.zookeeper:zookeeper in /athena-hbase
  - Migrate DynamoDB Connector to use AWS SDK V2 (#1810)
  - BUG 1728 : pom changes for bigquery connector (#1788)
  - Extended QPT to athena-docdb (#1796)
  - Extended QPT to athena-bigquery (#1795)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chngpe chngpe chngpe approved these changes

@aimethed aimethed aimethed approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@Trianz-Akshay @chngpe @aimethed