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

fix(elasticsearch) Analytics indices creation on AWS ES #5502

Merged
merged 17 commits into from
Sep 29, 2022
Merged

fix(elasticsearch) Analytics indices creation on AWS ES #5502

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
cdee86f
refactor(elasticsearch-setup-job): create-indices.sh readability
tomas-kubin Jul 27, 2022
2c61723
fix(elasticsearch-setup-job): AWS indices creation
tomas-kubin Jul 27, 2022
362c47c
feat(elasticsearch-setup-job): configuration hint
tomas-kubin Jul 28, 2022
2e1b5e6
fix(elasticsearch-setup-job): silent curl
tomas-kubin Jul 28, 2022
51dac7b
Merge branch 'master' into aws-es-indices-fix
tomas-kubin Aug 26, 2022
2cf8276
Merge branch 'master' into aws-es-indices-fix
tomas-kubin Aug 29, 2022
b7433f9
fix(elasticsearch-setup-job): better USE_AWS_ELASTICSEARCH hint
tomas-kubin Aug 30, 2022
750b2d0
docs(elasticsearch-setup-job): index dropping explained
tomas-kubin Aug 30, 2022
92c2673
fix(elasticsearch-setup-job): script fixes
tomas-kubin Aug 30, 2022
9236d2e
Merge branch 'master' into aws-es-indices-fix
tomas-kubin Aug 30, 2022
31ac516
Merge remote-tracking branch 'origin/master' into aws-es-indices-fix
tomas-kubin Sep 20, 2022
944984c
merge(elasticsearch-setup-job): merging in PR #5937
tomas-kubin Sep 20, 2022
e820132
merge(elasticsearch-setup-job): merging in PR #5963
tomas-kubin Sep 20, 2022
aa6cf0d
merge(elasticsearch-setup-job): merging in PR #5975
tomas-kubin Sep 20, 2022
71653b0
Merge remote-tracking branch 'origin/aws-es-indices-fix' into aws-es-…
tomas-kubin Sep 20, 2022
43fcec5
Merge branch 'master' into aws-es-indices-fix
tomas-kubin Sep 20, 2022
0873672
Merge branch 'master' into aws-es-indices-fix
pedro93 Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 72 additions & 47 deletions docker/elasticsearch-setup/create-indices.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,71 +5,96 @@ set -e
: ${DATAHUB_ANALYTICS_ENABLED:=true}
: ${USE_AWS_ELASTICSEARCH:=false}

# protocol: http or https?
if [[ $ELASTICSEARCH_USE_SSL == true ]]; then
ELASTICSEARCH_PROTOCOL=https
else
ELASTICSEARCH_PROTOCOL=http
fi

if [[ ! -z $ELASTICSEARCH_USERNAME ]] && [[ -z $ELASTICSEARCH_AUTH_HEADER ]]; then
AUTH_TOKEN=$(echo -ne "$ELASTICSEARCH_USERNAME:$ELASTICSEARCH_PASSWORD" | base64 --wrap 0)
ELASTICSEARCH_AUTH_HEADER="Authorization:Basic $AUTH_TOKEN"
fi
# Elasticsearch URL to be suffixed with a resource address
ELASTICSEARCH_URL="$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT"

# Add default header if needed
# set auth header if none is given
if [[ -z $ELASTICSEARCH_AUTH_HEADER ]]; then
ELASTICSEARCH_AUTH_HEADER="Accept: */*"
fi

function create_datahub_usage_event_datastream() {
if [[ -z "$INDEX_PREFIX" ]]; then
PREFIX=''
if [[ ! -z $ELASTICSEARCH_USERNAME ]]; then
# no auth header given, but username is defined -> use it to create the auth header
AUTH_TOKEN=$(echo -ne "$ELASTICSEARCH_USERNAME:$ELASTICSEARCH_PASSWORD" | base64 --wrap 0)
ELASTICSEARCH_AUTH_HEADER="Authorization:Basic $AUTH_TOKEN"
else
PREFIX="${INDEX_PREFIX}_"
# no auth header or username given -> use default auth header
ELASTICSEARCH_AUTH_HEADER="Accept: */*"
fi
fi

# index prefix used throughout the script
if [[ -z "$INDEX_PREFIX" ]]; then
PREFIX=''
else
PREFIX="${INDEX_PREFIX}_"
fi

# path where index definitions are stored
INDEX_DEFINITIONS_ROOT=/index/usage-event
Copy link
Collaborator

Choose a reason for hiding this comment

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

thank you!



# check Elasticsearch for given index/resource (first argument)
# if it doesn't exist (http code 404), use the given file (second argument) to create it
function create_if_not_exists {
RESOURCE_ADDRESS="$1"
RESOURCE_DEFINITION_NAME="$2"

# query ES to see if the resource already exists
RESOURCE_STATUS=$(curl -o /dev/null -s -w "%{http_code}\n" --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_URL/$RESOURCE_ADDRESS")

if [ $RESOURCE_STATUS -eq 200 ]; then
# resource already exists -> nothing to do
echo -e "\n>>> $RESOURCE_ADDRESS already exists ✓"

elif [ $RESOURCE_STATUS -eq 404 ]; then
# resource doesn't exist -> need to create it
echo -e "\n>>> creating $RESOURCE_ADDRESS ..."
# use the given path as definition, but first replace all occurences of PREFIX with the actual prefix
Copy link
Collaborator

Choose a reason for hiding this comment

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

these comments are super helper. thank you

TMP_SOURCE_PATH="/tmp/$RESOURCE_DEFINITION_NAME"
sed -e "s/PREFIX/$PREFIX/g" "$INDEX_DEFINITIONS_ROOT/$RESOURCE_DEFINITION_NAME" | tee -a "$TMP_SOURCE_PATH"
curl -s -XPUT --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_URL/$RESOURCE_ADDRESS" -H 'Content-Type: application/json' --data "@$TMP_SOURCE_PATH"

elif [ $RESOURCE_STATUS -eq 401 ] || [ $RESOURCE_STATUS -eq 405 ]; then
echo -e "\n>>> failed to GET $RESOURCE_ADDRESS ($RESOURCE_STATUS) !"
echo "... make sure you have correct USE_AWS_ELASTICSEARCH env value set (current=$USE_AWS_ELASTICSEARCH)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why AWS specific?

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 having USE_AWS_ELASTICSEARCH set to true is the common issue several people have met, which then causes Analytics to go wrong. I observed the 401/405 errors to be linked with this particular misconfiguration and figured it might be helpful for people to be aware of the obvious fix. (See the log in Testing / Case 2 section of the PR description.)

However I can see this looks too specific for a general-purpose function. Let me add a condition checking whether $ELASTICSEARCH_URL contains some aws-specific substring and display the message only then.

exit 1

if [ $(curl -o /dev/null -s -w "%{http_code}" --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_ilm/policy/${PREFIX}datahub_usage_event_policy") -eq 404 ]
then
echo -e "\ncreating datahub_usage_event_policy"
sed -e "s/PREFIX/${PREFIX}/g" /index/usage-event/policy.json | tee -a /tmp/policy.json
curl -XPUT --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_ilm/policy/${PREFIX}datahub_usage_event_policy" -H 'Content-Type: application/json' --data @/tmp/policy.json
else
echo -e "\ndatahub_usage_event_policy exists"
fi
if [ $(curl -o /dev/null -s -w "%{http_code}" --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_index_template/${PREFIX}datahub_usage_event_index_template") -eq 404 ]
then
echo -e "\ncreating datahub_usage_event_index_template"
sed -e "s/PREFIX/${PREFIX}/g" /index/usage-event/index_template.json | tee -a /tmp/index_template.json
curl -XPUT --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_index_template/${PREFIX}datahub_usage_event_index_template" -H 'Content-Type: application/json' --data @/tmp/index_template.json
else
echo -e "\ndatahub_usage_event_index_template exists"
echo -e "\n>>> unexpected $RESOURCE_ADDRESS status $RESOURCE_STATUS !"
exit 1
fi
}

# create indices for ES (non-AWS)
function create_datahub_usage_event_datastream() {
create_if_not_exists "_ilm/policy/${PREFIX}datahub_usage_event_policy" policy.json
create_if_not_exists "_index_template/${PREFIX}datahub_usage_event_index_template" index_template.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we not need a line like this here:

 create_if_not_exists "${PREFIX}datahub_usage_event" aws_es_usage_event.json

to ensure that the index is actually created for non aws cases?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Because I do think there's a separate issue actively open where on fresh quickstarts without any usage events, the analytics index will be missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how this works outside of AWS. The behavior in non-AWS environment should be the same as before refactoring.

Can you link the issue mentioned?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's this PR in question: #5974

}

# create indices for ES OSS (AWS)
function create_datahub_usage_event_aws_elasticsearch() {
if [[ -z "$INDEX_PREFIX" ]]; then
PREFIX=''
else
PREFIX="${INDEX_PREFIX}_"
fi
create_if_not_exists "_opendistro/_ism/policies/${PREFIX}datahub_usage_event_policy" aws_es_ism_policy.json
create_if_not_exists "_template/${PREFIX}datahub_usage_event_index_template" aws_es_index_template.json

if [ $(curl -o /dev/null -s -w "%{http_code}" --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_opendistro/_ism/policies/${PREFIX}datahub_usage_event_policy") -eq 404 ]
then
echo -e "\ncreating datahub_usage_event_policy"
sed -e "s/PREFIX/${PREFIX}/g" /index/usage-event/aws_es_ism_policy.json | tee -a /tmp/aws_es_ism_policy.json
curl -XPUT --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_opendistro/_ism/policies/${PREFIX}datahub_usage_event_policy" -H 'Content-Type: application/json' --data @/tmp/aws_es_ism_policy.json
else
echo -e "\ndatahub_usage_event_policy exists"
fi
if [ $(curl -o /dev/null -s -w "%{http_code}" --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_template/${PREFIX}datahub_usage_event_index_template") -eq 404 ]
then
echo -e "\ncreating datahub_usage_event_index_template"
sed -e "s/PREFIX/${PREFIX}/g" /index/usage-event/aws_es_index_template.json | tee -a /tmp/aws_es_index_template.json
curl -XPUT --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_template/${PREFIX}datahub_usage_event_index_template" -H 'Content-Type: application/json' --data @/tmp/aws_es_index_template.json
curl -XPUT --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/${PREFIX}datahub_usage_event-000001" -H 'Content-Type: application/json' --data "{\"aliases\":{\"${PREFIX}datahub_usage_event\":{\"is_write_index\":true}}}"
else
echo -e "\ndatahub_usage_event_index_template exists"
# this fixes the case when datahub_usage_event was created by GMS before datahub_usage_event-000001
USAGE_EVENT_STATUS=$(curl -o /dev/null -s -w "%{http_code}\n" --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_URL/${PREFIX}datahub_usage_event")
if [ $USAGE_EVENT_STATUS -eq 200 ]; then
USAGE_EVENT_DEFINITION=$(curl -s --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_URL/${PREFIX}datahub_usage_event")
# the definition is expected to contain "datahub_usage_event-000001" string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Consider adding to comment here)

if [[ $USAGE_EVENT_DEFINITION != *"datahub_usage_event-000001"* ]]; then
# ... if it doesn't, we need to drop it
echo -e "\n>>> deleting invalid datahub_usage_event ..."
curl -s -XDELETE --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_URL/${PREFIX}datahub_usage_event"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Deleting an index in the setup job seems dangerous. We need to be EXTREMELY careful that this doesn't affect existing deployments. Can you explain a bit more why this drop is necessary?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will analytics necessarily not work if the conditions is met to enter this block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if the index was previously created with USE_AWS_ELASTICSEARCH not set, and then later switched to true, Analytics tab doesn't work and no analytics events get ever recorded. (The name matches, but the contents do not.) The only solution (and the core fix of this PR) is to drop the wrong index and recreate it anew using aws_es_usage_event.json as source.

This block is executed only if USE_AWS_ELASTICSEARCH=true (we are on AWS) and only if there is an existing datahub_usage_event index which doesn't contain the AWS-specific part — indicating the case described above.

What could go wrong? Maybe if the name of the index is later changed and this condition is not adjusted appropriately. Let me extract it to a constant to be safe and add some more comments in the code.

Copy link
Collaborator

@RyanHolstien RyanHolstien Aug 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Dropping is probably too harsh here, we have an alias. If it's a mapping that needs to be fixed it should use the reindex API rather than a full drop of the index.

# ... and then recreate it below
fi
fi

create_if_not_exists "${PREFIX}datahub_usage_event-000001" aws_es_usage_event.json
}

if [[ $DATAHUB_ANALYTICS_ENABLED == true ]]; then
Expand Down
7 changes: 7 additions & 0 deletions ...-service/restli-servlet-impl/src/main/resources/index/usage-event/aws_es_usage_event.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
jjoyce0510 marked this conversation as resolved.
Show resolved Hide resolved
"aliases": {
"PREFIXdatahub_usage_event": {
"is_write_index": true
}
}
}